[PATCH] vfprintf: validate nargs and maybe allocate from heap

[Kees Cook]

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