This is the mail archive of the glibc-cvs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

GNU C Library master sources branch, master, updated. glibc-2.15-288-gfa03551


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  fa0355175d60ccf610c98f2345504603d3b8ea57 (commit)
       via  7c1f4834d398163d1ac8101e35e9c36fc3176e6e (commit)
      from  c6922934363f44b88250567f52036d8e9972c255 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sources.redhat.com/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=fa0355175d60ccf610c98f2345504603d3b8ea57

commit fa0355175d60ccf610c98f2345504603d3b8ea57
Author: Kees Cook <keescook@chromium.org>
Date:   Mon Mar 5 10:18:17 2012 +0100

    2012-03-02  Kees Cook  <keescook@chromium.org>
    
            * stdio-common/vfprintf.c (vfprintf): add missing errno settings.

diff --git a/ChangeLog b/ChangeLog
index dad26da..0fe4565 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 2012-03-02  Kees Cook  <keescook@chromium.org>
 
+        * stdio-common/vfprintf.c (vfprintf): add missing errno settings.
+
         [BZ #13656]
         * stdio-common/vfprintf.c (vfprintf): Check for nargs overflow and
         possibly allocate from heap instead of stack.
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index c802e46..85d1900 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -822,7 +822,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 									      \
 	if (function_done < 0)						      \
 	  {								      \
-	    /* Error in print handler.  */				      \
+	    /* Error in print handler; up to handler to set errno.  */	      \
 	    done = -1;							      \
 	    goto all_done;						      \
 	  }								      \
@@ -876,7 +876,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 									      \
 	if (function_done < 0)						      \
 	  {								      \
-	    /* Error in print handler.  */				      \
+	    /* Error in print handler; up to handler to set errno.  */	      \
 	    done = -1;							      \
 	    goto all_done;						      \
 	  }								      \
@@ -1117,7 +1117,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 			 &mbstate);					      \
 	if (len == (size_t) -1)						      \
 	  {								      \
-	    /* Something went wron gduring the conversion.  Bail out.  */     \
+	    /* Something went wrong during the conversion.  Bail out.  */     \
 	    done = -1;							      \
 	    goto all_done;						      \
 	  }								      \
@@ -1188,6 +1188,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 		      if (__mbsnrtowcs (ignore, &str2, strend - str2,	      \
 					ignore_size, &ps) == (size_t) -1)     \
 			{						      \
+			  /* Conversion function has set errno.  */	      \
 			  done = -1;					      \
 			  goto all_done;				      \
 			}						      \
@@ -1605,6 +1606,7 @@ vfprintf (FILE *s, const CHAR_T *format, va_list ap)
 	  if (spec == L_('\0'))
 	    {
 	      /* The format string ended before the specifier is complete.  */
+	      __set_errno (EINVAL);
 	      done = -1;
 	      goto all_done;
 	    }
@@ -1948,6 +1950,7 @@ do_positional:
 		       about # of chars.  */
 		    if (function_done < 0)
 		      {
+			/* Function has set errno.  */
 			done = -1;
 			goto all_done;
 		      }
@@ -1982,6 +1985,7 @@ do_positional:
 		 of chars.  */
 	      if (function_done < 0)
 		{
+		  /* Function has set errno.  */
 		  done = -1;
 		  goto all_done;
 		}

http://sources.redhat.com/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=7c1f4834d398163d1ac8101e35e9c36fc3176e6e

commit 7c1f4834d398163d1ac8101e35e9c36fc3176e6e
Author: Kees Cook <keescook@chromium.org>
Date:   Mon Mar 5 10:17:22 2012 +0100

    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.

diff --git a/ChangeLog b/ChangeLog
index 4cf6446..dad26da 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 a847b28..080badc 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 0000000..13c66c0
--- /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 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);

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                         |   10 +++++
 stdio-common/Makefile             |    3 +-
 stdio-common/bug-vfprintf-nargs.c |   78 +++++++++++++++++++++++++++++++++++++
 stdio-common/vfprintf.c           |   57 +++++++++++++++++++++------
 4 files changed, 135 insertions(+), 13 deletions(-)
 create mode 100644 stdio-common/bug-vfprintf-nargs.c


hooks/post-receive
-- 
GNU C Library master sources


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]