Re: Fix PR c++/19351 (operator new[] overflow)

[Florian Weimer <fweimer at redhat dot com>]

On 07/18/2012 03:55 PM, Jason Merrill wrote:
> On 06/26/2012 10:29 AM, Florian Weimer wrote:
> > +  /* Set to (size_t)-1 if the size check fails.  */
> > +  if (size_check != NULL_TREE)
> > +    *size = fold_build3 (COND_EXPR, sizetype, size_check,
> > +             original_size, TYPE_MAX_VALUE (sizetype));
> >     VEC_safe_insert (tree, gc, *args, 0, *size);
> >     *args = resolve_args (*args, complain);
> >     if (*args == NULL)
> > @@ -4022,7 +4030,11 @@ build_operator_new_call (tree fnname,
> > VEC(tree,gc) **args,
> >          if (use_cookie)
> >        {
> >          /* Update the total size.  */
> > -       *size = size_binop (PLUS_EXPR, *size, *cookie_size);
> > +       *size = size_binop (PLUS_EXPR, original_size, *cookie_size);
> > +       /* Set to (size_t)-1 if the size check fails.  */
> > +       gcc_assert (size_check != NULL_TREE);
> > +       *size = fold_build3 (COND_EXPR, sizetype, size_check,
> > +                *size, TYPE_MAX_VALUE (sizetype));
>
> Looks like you're evaluating the size_check twice for types that use
> cookies.

I try to avoid this by using original_size instead of size on the first 
assignment under the use_cookie case.

> > +      /* Unconditionally substract the array size.  This decreases the
> > +     maximum object size and is safe even if we choose not to use
> > +     a cookie after all.  */
>
> "cookie size"

Okay, I will fix that.

> But since we're going to be deciding whether or not to use a cookie in
> this function anyway, why not do it here?

The decision to use a cookie is currently split between the two 
functions and there are several special cases (Java, ABI compatibility 
flags).  I did not want to disturb this code too much because we do not 
have much test suite coverage in this area.

--
Florian Weimer / Red Hat Product Security Team