diff options
| -rw-r--r-- | ChangeLog | 8 | ||||
| -rw-r--r-- | stdio-common/Makefile | 3 | ||||
| -rw-r--r-- | stdio-common/bug-vfprintf-nargs.c | 78 | ||||
| -rw-r--r-- | stdio-common/vfprintf.c | 47 |
4 files changed, 126 insertions, 10 deletions
diff --git a/ChangeLog b/ChangeLog index 4cf6446135..dad26dad01 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2012-03-02 Kees Cook <keescook@chromium.org> + + [BZ #13656] + * stdio-common/vfprintf.c (vfprintf): Check for nargs overflow and + possibly allocate from heap instead of stack. + * stdio-common/bug-vfprintf-nargs.c: New file. + * stdio-common/Makefile (tests): Add nargs overflow test. + 2012-03-03 Andreas Schwab <schwab@linux-m68k.org> * sysdeps/powerpc/fpu/libm-test-ulps: Update. diff --git a/stdio-common/Makefile b/stdio-common/Makefile index a847b28fbc..080badc36c 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -59,7 +59,8 @@ tests := tstscanf test_rdwr test-popen tstgetln test-fseek \ tst-popen tst-unlockedio tst-fmemopen2 tst-put-error tst-fgets \ tst-fwrite bug16 bug17 tst-swscanf tst-sprintf2 bug18 bug18a \ bug19 bug19a tst-popen2 scanf13 scanf14 scanf15 bug20 bug21 bug22 \ - scanf16 scanf17 tst-setvbuf1 tst-grouping bug23 bug24 + scanf16 scanf17 tst-setvbuf1 tst-grouping bug23 bug24 \ + bug-vfprintf-nargs test-srcs = tst-unbputc tst-printf diff --git a/stdio-common/bug-vfprintf-nargs.c b/stdio-common/bug-vfprintf-nargs.c new file mode 100644 index 0000000000..13c66c0486 --- /dev/null +++ b/stdio-common/bug-vfprintf-nargs.c @@ -0,0 +1,78 @@ +/* Test for vfprintf nargs allocation overflow (BZ #13656). + Copyright (C) 2012 Free Software Foundation, Inc. + This file is part of the GNU C Library. + Contributed by Kees Cook <keescook@chromium.org>, 2012. + + 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, write to the Free + Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA + 02111-1307 USA. */ + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <unistd.h> +#include <inttypes.h> +#include <string.h> +#include <signal.h> + +static int +format_failed (const char *fmt, const char *expected) +{ + char output[80]; + + printf ("%s : ", fmt); + + memset (output, 0, sizeof output); + /* Having sprintf itself detect a failure is good. */ + if (sprintf (output, fmt, 1, 2, 3, "test") > 0 + && strcmp (output, expected) != 0) + { + printf ("FAIL (output '%s' != expected '%s')\n", output, expected); + return 1; + } + puts ("ok"); + return 0; +} + +static int +do_test (void) +{ + int rc = 0; + char buf[64]; + + /* Regular positionals work. */ + if (format_failed ("%1$d", "1") != 0) + rc = 1; + + /* Regular width positionals work. */ + if (format_failed ("%1$*2$d", " 1") != 0) + rc = 1; + + /* Positional arguments are constructed via read_int, so nargs can only + overflow on 32-bit systems. On 64-bit systems, it will attempt to + allocate a giant amount of memory and possibly crash, which is the + expected situation. Since the 64-bit behavior is arch-specific, only + test this on 32-bit systems. */ + if (sizeof (long int) == 4) + { + sprintf (buf, "%%1$d %%%" PRIdPTR "$d", UINT32_MAX / sizeof (int)); + if (format_failed (buf, "1 %$d") != 0) + rc = 1; + } + + return rc; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c index 863cd5d179..c802e46349 100644 --- a/stdio-common/vfprintf.c +++ b/stdio-common/vfprintf.c @@ -235,6 +235,9 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap) 0 if unknown. */ int readonly_format = 0; + /* For the argument descriptions, which may be allocated on the heap. */ + void *args_malloced = NULL; + /* This table maps a character into a number representing a class. In each step there is a destination label for each class. */ @@ -1647,9 +1650,10 @@ do_positional: determine the size of the array needed to store the argument attributes. */ size_t nargs = 0; - int *args_type; - union printf_arg *args_value = NULL; + size_t bytes_per_arg; + union printf_arg *args_value; int *args_size; + int *args_type; /* Positional parameters refer to arguments directly. This could also determine the maximum number of arguments. Track the @@ -1698,13 +1702,38 @@ do_positional: /* Determine the number of arguments the format string consumes. */ nargs = MAX (nargs, max_ref_arg); + /* Calculate total size needed to represent a single argument across + all three argument-related arrays. */ + bytes_per_arg = sizeof (*args_value) + sizeof (*args_size) + + sizeof (*args_type); + + /* Check for potential integer overflow. */ + if (__builtin_expect (nargs > SIZE_MAX / bytes_per_arg, 0)) + { + __set_errno (ERANGE); + done = -1; + goto all_done; + } - /* Allocate memory for the argument descriptions. */ - args_type = alloca (nargs * sizeof (int)); + /* Allocate memory for all three argument arrays. */ + if (__libc_use_alloca (nargs * bytes_per_arg)) + args_value = alloca (nargs * bytes_per_arg); + else + { + args_value = args_malloced = malloc (nargs * bytes_per_arg); + if (args_value == NULL) + { + done = -1; + goto all_done; + } + } + + /* Set up the remaining two arrays to each point past the end of the + prior array, since space for all three has been allocated now. */ + args_size = &args_value[nargs].pa_int; + args_type = &args_size[nargs]; memset (args_type, s->_flags2 & _IO_FLAGS2_FORTIFY ? '\xff' : '\0', - nargs * sizeof (int)); - args_value = alloca (nargs * sizeof (union printf_arg)); - args_size = alloca (nargs * sizeof (int)); + nargs * sizeof (*args_type)); /* XXX Could do sanity check here: If any element in ARGS_TYPE is still zero after this loop, format is invalid. For now we @@ -1973,8 +2002,8 @@ do_positional: } all_done: - if (__builtin_expect (workstart != NULL, 0)) - free (workstart); + free (args_malloced); + free (workstart); /* Unlock the stream. */ _IO_funlockfile (s); _IO_cleanup_region_end (0); |