Hi Ross,
unfortunately I could not find any packet that allowed me to reproduce
the issue (feeding hard coded data to processIncomingReport with info
I'd collected from memory right before the crash).
What looks like a quick fix to me is the following:
there is a code line in RTCP.cpp::processIncomingReport which checks if
the remaining "packetSize" is smaller than the next chunk (defined by
"length") => if (length > packetSize) break;
If so, the loop exits and so the call to processIncomingReport.
It seems to me that this check was to prevent faulty processing or
input, though I believe in my case this is insufficient.
If "packetSize" is able to drop below zero (effectively making it a very
big number as it is unsigned) then "length" always has a corrupted value
because it is derived from "pkt" which is invalid since "packetSize" is < 0.
Because "packetSize" is then over four billion (it dropped below zero),
length is very likely to be smaller, allowing the code to continue
execution and to go rampage reading from random memory.
The check would have been ok if packetSize would be of a signed type,
allowing it to be smaller than zero.
My suggestion is to replace "#define ADVANCE(n) pkt += (n); packetSize
-= (n);" with "#define ADVANCE(n) if (! okPkt(pkt, totPacketSize, n))
break; pkt += (n); packetSize -= (n);"
and to add "okPkt":
bool RTCPInstance::okPkt(const unsigned char* const pkt, const unsigned
totPacketSize, const int n) const
{
const u_int8_t* const nextPkt = pkt + n;
if ( nextPkt < fInBuf || nextPkt >= (fInBuf + maxRTCPPacketSize) )
{
printf("index ptr out of array bounds at incoming RTCP packet");
return false;
}
else if ( nextPkt >= (fInBuf + totPacketSize) )
{
printf("index ptr out of packet bounds at incoming RTCP packet");
return false;
}
return true;
}
Combining a memory range check with pointer arithmetic is a lot safer
than relying on expected input.
I'll send through more info about what triggers this if I find any.
Regards,
Frederik De Ruyck
Op 14/11/2016 om 09:54 schreef Frederik De Ruyck:
Hi,
Last week I've experienced a crash at:
void RTCPInstance::processIncomingReport(unsigned packetSize
, struct sockaddr_in const& fromAddressAndPort
, int tcpSocketNum
, unsigned char tcpStreamChannelId)
at line:
rtcpHdr = ntohl(*(u_int32_t*)pkt);
I've upgraded to the last version of Live555 (06-11-2016) to confirm
that the issue is still present.
I've had this crash three times with version 07-08-2016 and now today
a fourth time with latest 06-11-2016.
I've debugged the code and the crash is of type segfault because the
memory dereferenced at address pkt is likely outside the application's
memory space.
This is caused because packetSize is decremented beyond 0, the value
of "packetSize" at the time of the crash is 4294068404, it is of
unsigned type so it overflows to a huge number when it drops below 0.
It is the macro ADVANCE(length) that causes the pointer "pkt" to refer
to an address that is beyond the scope of "fInBuf_ptr".
I will now try to copy the incoming packet contents out of my debugger
memory editor and see if I can create a test case with that.
Regards,
Frederik De Ruyck
This email has been scanned by BullGuard antivirus protection.
For more info visit www.bullguard.com
___
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel
This email has been scanned by BullGuard antivirus protection.
For more info visit www.bullguard.com
___
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel
