forked from pool/glibc
Accepting request 107799 from home:a_jaeger:my-factory-packages
vfprintf: validate nargs and maybe allocate from heap (bnc#747768) OBS-URL: https://build.opensuse.org/request/show/107799 OBS-URL: https://build.opensuse.org/package/show/Base:System/glibc?expand=0&rev=162
This commit is contained in:
parent
217e7cdd40
commit
392fec9468
@ -1,3 +1,8 @@
|
||||
-------------------------------------------------------------------
|
||||
Fri Mar 2 17:01:55 UTC 2012 - aj@suse.de
|
||||
|
||||
- vfprintf: validate nargs and maybe allocate from heap (bnc#747768)
|
||||
|
||||
-------------------------------------------------------------------
|
||||
Tue Jan 31 13:20:37 UTC 2012 - aj@suse.de
|
||||
|
||||
|
@ -206,6 +206,8 @@ Patch91: glibc-ifunc-2.16.patch
|
||||
Patch92: glibc-revert-netlink-cache.patch
|
||||
# PATCH-FIX-UPSTREAM Fix tests for AVX features - aj@suse.de
|
||||
Patch93: glibc-2.16-avx.patch
|
||||
# PATCH-FIX-UPSTREAM vfprintf: validate nargs and maybe allocate from heap bnc#747768 - aj@suse.de
|
||||
Patch94: vfprintf-nargs.patch
|
||||
|
||||
%description
|
||||
The GNU C Library provides the most important standard libraries used
|
||||
@ -441,6 +443,7 @@ rm nscd/s-stamp
|
||||
%patch91 -p1
|
||||
%patch92 -p1 -R
|
||||
%patch93 -p1
|
||||
%patch94 -p1
|
||||
|
||||
#
|
||||
# Inconsistency detected by ld.so: dl-close.c: 719: _dl_close: Assertion `map->l_init_called' failed!
|
||||
|
207
vfprintf-nargs.patch
Normal file
207
vfprintf-nargs.patch
Normal file
@ -0,0 +1,207 @@
|
||||
Date: Thu, 16 Feb 2012 08:16:13 -0800
|
||||
From: Kees Cook <kees@outflux.net>
|
||||
To: "Ryan S. Arnold" <ryan.arnold@gmail.com>
|
||||
Cc: libc-alpha@sourceware.org, Paul Eggert <eggert@cs.ucla.edu>,
|
||||
Roland McGrath <roland@hack.frob.com>,
|
||||
Andreas Schwab <schwab@linux-m68k.org>
|
||||
Subject: Re: [PATCH] vfprintf: validate nargs and maybe allocate from heap
|
||||
|
||||
The nargs value can overflow when doing allocations, allowing arbitrary
|
||||
memory writes via format strings, bypassing _FORTIFY_SOURCE:
|
||||
http://www.phrack.org/issues.html?issue=67&id=9
|
||||
|
||||
This checks for nargs overflow and possibly allocates from heap instead of
|
||||
stack, and adds a regression test for the situation.
|
||||
|
||||
I have FSF assignment via Google. (Sent from @outflux since that's how I'm
|
||||
subscribed here, but CL shows @chromium.org as part of my Google work.)
|
||||
|
||||
This version disables the useless test on non-32-bit platforms.
|
||||
|
||||
2012-02-16 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/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..022e72b 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,33 @@ do_positional:
|
||||
|
||||
/* Determine the number of arguments the format string consumes. */
|
||||
nargs = MAX (nargs, max_ref_arg);
|
||||
+ bytes_per_arg = sizeof (*args_value) + sizeof (*args_size)
|
||||
+ + sizeof (*args_type);
|
||||
+
|
||||
+ /* Check for potential integer overflow. */
|
||||
+ if (nargs > SIZE_MAX / bytes_per_arg)
|
||||
+ {
|
||||
+ done = -1;
|
||||
+ goto all_done;
|
||||
+ }
|
||||
|
||||
/* Allocate memory for the argument descriptions. */
|
||||
- args_type = alloca (nargs * sizeof (int));
|
||||
+ 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;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
+ 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 +1997,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);
|
||||
--
|
||||
1.7.5.4
|
||||
|
||||
--
|
||||
Kees Cook @outflux.net
|
Loading…
Reference in New Issue
Block a user