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

[Andreas Jaeger]

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