[26535] in bugtraq
Re: FreeBSD Security Advisory FreeBSD-SA-02:34.rpc
daemon@ATHENA.MIT.EDU (Casper Dik)
Mon Aug 5 16:39:11 2002
Message-Id: <200208040945.LAA29896@romulus.Holland.Sun.COM>
To: Adam Sampson <azz@gnu.org>
In-reply-to: Your message of "01 Aug 2002 09:31:10 BST."
<y2avg6vq8tt.fsf@cartman.at.fivegeeks.net>
Date: Sun, 04 Aug 2002 11:45:57 +0200
From: 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
