[reSIProcate] stunParseAtrError potential memory corruption

[Scott Godin]

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 at logitech.com>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 at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20101221/65b8ff8e/attachment.htm>