[Live-devel] Fix for a possible buffer overflow in SDESItem

[Jeremy Noring]

On Thu, Mar 11, 2010 at 8:43 AM, Jeremy Noring <jnoring at logitech.com> wrote:

> In RTCP.cpp,
>
> SDESItem::SDESItem(unsigned char tag, unsigned char const* value) {
>   unsigned length = strlen((char const*)value);
>   if (length > 511) length = 511;
>
>   fData[0] = tag;
>   fData[1] = (unsigned char)length;
>   memmove(&fData[2], value, length);
>
>   // Pad the trailing bytes to a 4-byte boundary:
>   while ((length)%4 > 0) fData[2 + length++] = '\0';
> }
>
> length is 511 bytes, and the additional padding at the end tacks on up to 3
> more bytes.  However, the buffer to which memory is being written to in
> RTCP.hh is declared as:
>
> unsigned char fData[2 + 0xFF]; // first 2 bytes are tag and length
>
> ...which is only 257 bytes.  It needs to be at least 515 bytes in length
> (511+ 2 is 513; padded to before the nearest 4 byte boundary bumps that up
> to 515).  So that line should change to:
>
> unsigned char fData[2 + 513]; // first 2 bytes are tag and length
>
> Either that, or the upper bound on "length" in the constructor needs to be
> shortened. (to work with 2 + 0xFF, length would need to be no greater than
> 251)
>
> Thanks,
>
> Jeremy
>

Actually, on second glance, the only realistic option is to shorten length,
because only a single byte is allotted to the size field in fData[1].  (note
that length is cast to unsigned char).  So in RTCP.cpp, I'd change this
line:

if (length > 251) length = 251;

-Jeremy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.live555.com/pipermail/live-devel/attachments/20100311/7870e29c/attachment-0001.html>