Re: [reSIProcate] stunParseAtrError potential memory corruption
Not sure who wrote the original - but I stole this class and modified it for the reTurn server/client code (renamed to StunMessage.cxx). Looking at the new code, I completely re-wrote this fn so that we were not limited to a maximum error reason in the same way as the Stun.cxx code does, and my new code does not have this problem.
As for Stun.cxx - I agree with your analysis below and will commit the fix.
Thanks Kennard!
Scott
On Mon, Dec 20, 2010 at 10:26 PM, Kennard White
<kennard_white@xxxxxxxxxxxx> wrote:
Hi,
We ran a static code analyizer over portions of the resip code base, and it found a likely problem in rutil/stun/Stun.cxx, in stunParseAtrError().
The comparison if ( hdrLen >= sizeof(result) ) is too loose, I think because of the sizeReason field added to the end of the struct.
Specifically, sizeof(result) is 262, thus hdrLen can be 261, which means sizeReason can be as large as 257, which means it will try copying 257 bytes into a 256 byte buffer.
Proposed change would be:
if ( hdrLen >= sizeof(result)-2 )
But I don't know enough about the protocol specifics to know if this is the correct change. Does anyone currently "own" this code?
Regards,
Kennard
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel