This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] vfprintf: validate nargs and maybe allocate from heap
- From: Andreas Jaeger <aj at suse dot com>
- To: Kees Cook <kees at outflux dot net>
- Cc: "Ryan S. Arnold" <ryan dot arnold at gmail dot com>,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>
- Date: Fri, 02 Mar 2012 19:19:41 +0100
- Subject: Re: [PATCH] vfprintf: validate nargs and maybe allocate from heap
- References: <20120206062537.GM4979@outflux.net> <20120207000509.GP4989@outflux.net> <20120210192457.GF20420@outflux.net> <CAAKybw8AgkGsKAx=kvX4Tsi74f+HtuVnatTCB0VfsHi7vVFi1Q@mail.gmail.com> <20120214223048.GM20420@outflux.net> <CAAKybw_HS+cav+YcDw3ns7UXu6_xA7EHPrkiB87P+OGwEB0PVQ@mail.gmail.com> <20120214224543.GN20420@outflux.net> <20120216161613.GZ20420@outflux.net> <4F50EE1A.90902@suse.com> <20120302164858.GB3990@outflux.net>
On 03/02/2012 05:48 PM, Kees Cook wrote:
Hi Andreas,
On Fri, Mar 02, 2012 at 04:58:18PM +0100, Andreas Jaeger wrote:
On 02/16/2012 05:16 PM, Kees Cook wrote:
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
So a security issue - can we get this fixed quickly, please? I'd
like to ping for a review and commit!
Ryan has been trying to make some time for a final testing round, so
I'm confident a commit will be coming soon.
Ryan, do you see any problems or want specific tests? I just tested on
x86 and x86-64 and think the patch is fine to commit. I can do the
commit, just tell me what's holding you up...
Kees, thanks for the patch.
Sure thing!
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
[...]
@@ -1698,13 +1702,33 @@ do_positional:
+ bytes_per_arg = sizeof (*args_value) + sizeof (*args_size)
+ + sizeof (*args_type);
...
+ 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);
...
+ }
+
+ args_size =&args_value[nargs].pa_int;
+ args_type =&args_size[nargs];
don't you have an off-by-one error here? You allocate nargs
arguments and access [nargs]
This is a bit of type trickery. The allocation covers all three arrays,
args_value, args_size, and args_type. This code is setting up the other
two pointers to aim just after where the previous array ends. So, yes,
it is "past the end", but intentionally so.
That makes sense - could you add a comment to explain this?
Thanks,
Andreas
--
Andreas Jaeger aj@{suse.com,opensuse.org} Twitter/Identica: jaegerandi
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn,Jennifer Guild,Felix Imendörffer,HRB16746 (AG Nürnberg)
GPG fingerprint = 93A3 365E CE47 B889 DF7F FED1 389A 563C C272 A126