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.

1 files changed, 38 insertions, 9 deletions

diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index 863cd5d..c802e46 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);