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: Kees Cook <kees at outflux dot net>
- To: Andreas Jaeger <aj at suse dot com>
- 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, 2 Mar 2012 08:48:58 -0800
- 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>
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.
> 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.
-Kees
--
Kees Cook @outflux.net