Re: Segementation fault in onGotSDES

David,

That would be great. Then it can be included the next Suse beta release.

Regards,
Michel


David Sugar wrote:
> As soon as federico has a chance to also look at it, I could get an 
> updated release together.
> 
> David
> 
> Michel de Boer wrote:
> 
>> 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);
>>      }
>>         
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Ccrtp-devel mailing list
>> [email protected]
>> http://lists.gnu.org/mailman/listinfo/ccrtp-devel