Bugtraq: Re: FreeBSD Security Advisory FreeBSD-SA-02:34.rpc

[Casper Dik <Casper.Dik () Sun COM>]

> The FreeBSD patch says:
>
> >         c = *sizep;
> > -       if ((c > maxsize) && (xdrs->x_op != XDR_FREE)) {
> > +       if ((c > maxsize && UINT_MAX/elsize < c) &&
> > +           (xdrs->x_op != XDR_FREE)) {
> >                 return (FALSE);
> >         }
> Is this fix correct? Previously, xdr_array would return false if the
> count of items passed in was larger than the maximum; now it only
> returns false if it's both larger than the maximum _and_ larger than
> the amount that can be safely calculated. In the event that *sizep >
> maxsize but *sizep <= UINT_MAX/elsize, the return (FALSE) will never
> be hit, whereas it would be in the original version of the
> code. Shouldn't the first && be ||?
That's what we did for Solaris, so I guess you're right.

Our code has:

        if ((c > maxsize || LASTUNSIGNED / elsize < c) &&

(it was fixed in the style of the RPC code which predates UINT_MAX et.al.;
the code was never touched since).

Note that the trivial case of elsize == 1 is always safe as that's the
one case where no overflow will occur.

Casper