Message ID | alpine.DEB.2.21.2007081759570.15347@digraph.polyomino.org.uk |
---|---|
State | New |
Headers | show |
Series | Fix double free in __printf_fp_l (bug 26214) | expand |
On 08/07/2020 15:01, Joseph Myers wrote: > __printf_fp_l has a double free bug in the case where it allocates > memory with malloc internally, then has an I/O error while outputting > trailing padding and tries to free that already-freed memory when the > error occurs. This patch fixes this by setting the relevant pointer > to NULL after the first free (the only free of this pointer that isn't > immediately followed by returning from the function). > > Tested for x86_64 and x86. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > > diff --git a/stdio-common/Makefile b/stdio-common/Makefile > index 73bf0da296..dc3fd38a19 100644 > --- a/stdio-common/Makefile > +++ b/stdio-common/Makefile > @@ -67,7 +67,8 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \ > tst-renameat2 tst-bz11319 tst-bz11319-fortify2 \ > scanf14a scanf16a \ > tst-printf-bz25691 \ > - tst-vfprintf-width-prec-alloc > + tst-vfprintf-width-prec-alloc \ > + tst-printf-fp-free > > > test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble Ok. > @@ -78,11 +79,13 @@ tests-special += $(objpfx)tst-unbputc.out $(objpfx)tst-printf.out \ > $(objpfx)tst-setvbuf1-cmp.out \ > $(objpfx)tst-vfprintf-width-prec-mem.out \ > $(objpfx)tst-printfsz-islongdouble.out \ > - $(objpfx)tst-printf-bz25691-mem.out > + $(objpfx)tst-printf-bz25691-mem.out \ > + $(objpfx)tst-printf-fp-free-mem.out > generated += tst-printf-bz18872.c tst-printf-bz18872.mtrace \ > tst-printf-bz18872-mem.out \ > tst-vfprintf-width-prec.mtrace tst-vfprintf-width-prec-mem.out \ > - tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out > + tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out \ > + tst-printf-fp-free.mtrace tst-printf-fp-free-mem.out > endif > > tests-special += $(objpfx)tst-errno-manual.out > @@ -108,6 +111,8 @@ tst-vfprintf-width-prec-ENV = \ > MALLOC_TRACE=$(objpfx)tst-vfprintf-width-prec.mtrace > tst-printf-bz25691-ENV = \ > MALLOC_TRACE=$(objpfx)tst-printf-bz25691.mtrace > +tst-printf-fp-free-ENV = \ > + MALLOC_TRACE=$(objpfx)tst-printf-fp-free.mtrace > > $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc > $(SHELL) $< $(common-objpfx) '$(test-program-prefix)' > $@; \ Ok. > diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c > index 9e0ce962f2..49c693575e 100644 > --- a/stdio-common/printf_fp.c > +++ b/stdio-common/printf_fp.c > @@ -1250,6 +1250,9 @@ __printf_fp_l (FILE *fp, locale_t loc, > { > free (buffer); > free (wbuffer); > + /* Avoid a double free if the subsequent PADN encounters an > + I/O error. */ > + wbuffer = NULL; > } > } > Ok. > diff --git a/stdio-common/tst-printf-fp-free.c b/stdio-common/tst-printf-fp-free.c > new file mode 100644 > index 0000000000..fea52248e0 > --- /dev/null > +++ b/stdio-common/tst-printf-fp-free.c > @@ -0,0 +1,37 @@ > +/* Test double free bug in __printf_fp_l (bug 26214). > + Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <mcheck.h> > +#include <stdio.h> > +#include <sys/resource.h> > +#include <support/check.h> > + > +static int > +do_test (void) > +{ > + mtrace (); > + FILE *fp = fopen ("/dev/full", "w"); > + TEST_VERIFY_EXIT (fp != NULL); > + char buf[131072]; > + TEST_VERIFY_EXIT (setvbuf (fp, buf, _IOFBF, sizeof buf) == 0); > + TEST_COMPARE (fprintf (fp, "%-1000000.65536f", 1.0), -1); > + fclose (fp); > + return 0; > +} > + > +#include <support/test-driver.c> > Ok.
* Joseph Myers: > __printf_fp_l has a double free bug in the case where it allocates > memory with malloc internally, then has an I/O error while outputting > trailing padding and tries to free that already-freed memory when the > error occurs. This patch fixes this by setting the relevant pointer > to NULL after the first free (the only free of this pointer that isn't > immediately followed by returning from the function). Same question here: Should we treat this as a security bug? Thanks, Florian
diff --git a/stdio-common/Makefile b/stdio-common/Makefile index 73bf0da296..dc3fd38a19 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -67,7 +67,8 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \ tst-renameat2 tst-bz11319 tst-bz11319-fortify2 \ scanf14a scanf16a \ tst-printf-bz25691 \ - tst-vfprintf-width-prec-alloc + tst-vfprintf-width-prec-alloc \ + tst-printf-fp-free test-srcs = tst-unbputc tst-printf tst-printfsz-islongdouble @@ -78,11 +79,13 @@ tests-special += $(objpfx)tst-unbputc.out $(objpfx)tst-printf.out \ $(objpfx)tst-setvbuf1-cmp.out \ $(objpfx)tst-vfprintf-width-prec-mem.out \ $(objpfx)tst-printfsz-islongdouble.out \ - $(objpfx)tst-printf-bz25691-mem.out + $(objpfx)tst-printf-bz25691-mem.out \ + $(objpfx)tst-printf-fp-free-mem.out generated += tst-printf-bz18872.c tst-printf-bz18872.mtrace \ tst-printf-bz18872-mem.out \ tst-vfprintf-width-prec.mtrace tst-vfprintf-width-prec-mem.out \ - tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out + tst-printf-bz25691.mtrace tst-printf-bz25691-mem.out \ + tst-printf-fp-free.mtrace tst-printf-fp-free-mem.out endif tests-special += $(objpfx)tst-errno-manual.out @@ -108,6 +111,8 @@ tst-vfprintf-width-prec-ENV = \ MALLOC_TRACE=$(objpfx)tst-vfprintf-width-prec.mtrace tst-printf-bz25691-ENV = \ MALLOC_TRACE=$(objpfx)tst-printf-bz25691.mtrace +tst-printf-fp-free-ENV = \ + MALLOC_TRACE=$(objpfx)tst-printf-fp-free.mtrace $(objpfx)tst-unbputc.out: tst-unbputc.sh $(objpfx)tst-unbputc $(SHELL) $< $(common-objpfx) '$(test-program-prefix)' > $@; \ diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c index 9e0ce962f2..49c693575e 100644 --- a/stdio-common/printf_fp.c +++ b/stdio-common/printf_fp.c @@ -1250,6 +1250,9 @@ __printf_fp_l (FILE *fp, locale_t loc, { free (buffer); free (wbuffer); + /* Avoid a double free if the subsequent PADN encounters an + I/O error. */ + wbuffer = NULL; } } diff --git a/stdio-common/tst-printf-fp-free.c b/stdio-common/tst-printf-fp-free.c new file mode 100644 index 0000000000..fea52248e0 --- /dev/null +++ b/stdio-common/tst-printf-fp-free.c @@ -0,0 +1,37 @@ +/* Test double free bug in __printf_fp_l (bug 26214). + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <mcheck.h> +#include <stdio.h> +#include <sys/resource.h> +#include <support/check.h> + +static int +do_test (void) +{ + mtrace (); + FILE *fp = fopen ("/dev/full", "w"); + TEST_VERIFY_EXIT (fp != NULL); + char buf[131072]; + TEST_VERIFY_EXIT (setvbuf (fp, buf, _IOFBF, sizeof buf) == 0); + TEST_COMPARE (fprintf (fp, "%-1000000.65536f", 1.0), -1); + fclose (fp); + return 0; +} + +#include <support/test-driver.c>