Re: [Ccrtp-devel] Segementation fault in onGotSDES

[Michel de Boer]

David, Federico,

After some extensive debugging I managed to find the cause
for the segmentation fault. It took me several sleepless hours :-)

In QueueRTCPManager::takeInControlPacket() you traverse the
packets in a compound RTCP message, but at several places there
are no checks to see if the pointer has advanced beyond the last
packet and the code starts to access random memory.

Attached you find my patch on file control.cpp for the problem.

It seems I have just been 'lucky' that the bug did not hit me
before. Though I think I might have seen it before, but could never
reproduce it so often as I can now (it only hits me on one of my
PC's. On another PC it never shows up).

I had planned to release a version of my softphone tomorrow (if the
rest of my testing goes well :-) ). To release my softphone,
I would need to publish my private version of ccRTP with this patch
to avoid the segmentation faults. I prefer to rely on an
official release of ccRTP though.

When do you plan to release a next release of ccRTP including this
fix?

Best regards,
Michel

--- ccrtp-1.3.2/src/control.cpp 2005-05-02 19:27:04.000000000 +0200
+++ ccrtp-1.3.2b/src/control.cpp        2005-07-23 14:40:48.530213224 +0200
@@ -334,7 +334,7 @@
        }
 
        // Process all RR reports.
-       while ( (RTCPPacket::tRR == pkt->fh.type) ) {
+       while ( pointer < len && (RTCPPacket::tRR == pkt->fh.type) ) {
                sourceLink = getSourceBySSRC(pkt->getSSRC(),
                                             source_created);
                if ( checkSSRCInRTCPPkt(*sourceLink,source_created,
@@ -348,7 +348,8 @@
        // SDES, APP and BYE. process first everything but the
        // BYE packets.
        bool cname_found = false;
-       while ( (pkt->fh.type == RTCPPacket::tSDES ||
+       while ( pointer < len &&
+               (pkt->fh.type == RTCPPacket::tSDES ||
                 pkt->fh.type == RTCPPacket::tAPP) ) {
                I ( cname_found || !pkt->fh.padding );
                sourceLink = getSourceBySSRC(pkt->getSSRC(),
@@ -358,16 +359,15 @@
                                        transport_port) ) {
                        if ( pkt->fh.type == RTCPPacket::tSDES ) {
                                bool cname = onGotSDES(*s,*pkt);
-                               pointer += pkt->getLength();
                                cname_found = cname_found? cname_found : cname;
                        } else if ( pkt->fh.type == RTCPPacket::tAPP ) {
                                onGotAPP(*s,pkt->info.APP,pkt->getLength());
-                               pointer += pkt->getLength();
                        } else {
                                // error?
                        }
                }
                // Get the next packet in the compound.
+               pointer += pkt->getLength();
                pkt = reinterpret_cast<RTCPPacket *>(rtcpRecvBuffer +pointer);
        }