[glibc] - Avoid "nargs" integer overflow which could be used to bypass FORTIFY_SOURCE (#794797)
Jeffrey Law
law at fedoraproject.org
Tue Feb 21 04:21:39 UTC 2012
commit 0e190d479d2e7dd365577ff0d47169f851d34c24
Author: Jeff Law <law at redhat.com>
Date: Mon Feb 20 21:21:30 2012 -0700
- Avoid "nargs" integer overflow which could be used to bypass
FORTIFY_SOURCE (#794797)
glibc-rh794797.patch | 240 ++++++++++++++++++++++++++++++++++++++++++++++++++
glibc.spec | 8 ++-
2 files changed, 247 insertions(+), 1 deletions(-)
---
diff --git a/glibc-rh794797.patch b/glibc-rh794797.patch
new file mode 100644
index 0000000..271538d
--- /dev/null
+++ b/glibc-rh794797.patch
@@ -0,0 +1,240 @@
+From libc-alpha-return-25252-listarch-libc-alpha=sources dot redhat dot com at sourceware dot org Thu Feb 16 16:21:17 2012
+Return-Path: <libc-alpha-return-25252-listarch-libc-alpha=sources dot redhat dot com at sourceware dot org>
+Delivered-To: listarch-libc-alpha at sources dot redhat dot com
+Received: (qmail 5187 invoked by alias); 16 Feb 2012 16:21:14 -0000
+Delivered-To: moderator for libc-alpha at sourceware dot org
+Received: (qmail 2174 invoked by uid 22791); 16 Feb 2012 16:17:18 -0000
+X-SWARE-Spam-Status: No, hits=-2.0 required=5.0
+ tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,TW_TV,TW_VB,TW_VF,T_RP_MATCHES_RCVD
+X-Spam-Check-By: sourceware.org
+Date: Thu, 16 Feb 2012 08:16:13 -0800
+From: Kees Cook <kees at outflux dot net>
+To: "Ryan S dot Arnold" <ryan dot arnold at gmail dot com>
+Cc: libc-alpha at sourceware dot org, Paul Eggert <eggert at cs dot ucla dot edu>,
+ Roland McGrath <roland at hack dot frob dot com>,
+ Andreas Schwab <schwab at linux-m68k dot org>
+Subject: Re: [PATCH] vfprintf: validate nargs and maybe allocate from heap
+Message-ID: <20120216161613.GZ20420 at outflux.net>
+References: <20120206062537.GM4979 at outflux.net>
+ <20120207000509 dot GP4989 at outflux dot net>
+ <20120210192457 dot GF20420 at outflux dot net>
+ <CAAKybw8AgkGsKAx=kvX4Tsi74f+HtuVnatTCB0VfsHi7vVFi1Q at mail dot gmail dot com>
+ <20120214223048 dot GM20420 at outflux dot net>
+ <CAAKybw_HS+cav+YcDw3ns7UXu6_xA7EHPrkiB87P+OGwEB0PVQ at mail dot gmail dot com>
+ <20120214224543 dot GN20420 at outflux dot net>
+MIME-Version: 1.0
+Content-Type: text/plain; charset=us-ascii
+Content-Disposition: inline
+In-Reply-To: <20120214224543 dot GN20420 at outflux dot net>
+X-MIMEDefang-Filter: outflux$Revision: 1.316 $
+X-HELO: www.outflux.net
+Mailing-List: contact libc-alpha-help at sourceware dot org; run by ezmlm
+Precedence: bulk
+List-Id: <libc-alpha.sourceware.org>
+List-Subscribe: <mailto:libc-alpha-subscribe at sourceware dot org>
+List-Archive: <http://sourceware.org/ml/libc-alpha/>
+List-Post: <mailto:libc-alpha at sourceware dot org>
+List-Help: <mailto:libc-alpha-help at sourceware dot org>, <http://sourceware dot org/ml/#faqs>
+Sender: libc-alpha-owner at sourceware dot org
+Delivered-To: mailing list libc-alpha at sourceware dot org
+
+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 at 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 at 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
+
diff --git a/glibc.spec b/glibc.spec
index 1dad96f..00dea67 100644
--- a/glibc.spec
+++ b/glibc.spec
@@ -28,7 +28,7 @@
Summary: The GNU libc libraries
Name: glibc
Version: %{glibcversion}
-Release: 22%{?dist}
+Release: 23%{?dist}
# GPLv2+ is used in a bunch of programs, LGPLv2+ is used for libraries.
# Things that are linked directly into dynamically linked programs
# and shared libraries (e.g. crt files, lib*_nonshared.a) have an additional
@@ -98,6 +98,8 @@ Patch31 : %{name}-rh697149.patch
Patch32 : %{name}-rh739743.patch
# Discussion started upstream, patch needs to be submitted
Patch33 : %{name}-rh789238.patch
+# Patch posted upstream, discussion ongoing, Paul E. seems to think it's OK
+Patch34 : %{name}-rh794797.patch
Buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
@@ -350,6 +352,7 @@ rm -rf %{glibcportsdir}
%patch31 -p1
%patch32 -p1
%patch33 -p1
+%patch34 -p1
# A lot of programs still misuse memcpy when they have to use
# memmove. The memcpy implementation below is not tolerant at
@@ -1202,6 +1205,9 @@ rm -f *.filelist*
%endif
%changelog
+* Mon Feb 20 2012 Jeff Law <law at redhat.com> - 2.15-23
+ - Avoid "nargs" integer overflow which could be used to bypass FORTIFY_SOURCE (#794797)
+
* Mon Feb 20 2012 Jeff Law <law at redhat.com> - 2.15-22
- Fix main arena locking in malloc/calloc retry path (#789238)
More information about the scm-commits
mailing list