[reSIProcate] bug in SipFrag.cxx

[Byron Campen]

Does this look sane? Is it important to check whether the primary  
mimetype is message?

Index: resip/stack/Contents.cxx
===================================================================
--- resip/stack/Contents.cxx    (revision 6561)
+++ resip/stack/Contents.cxx    (working copy)
@@ -148,6 +148,14 @@
     assert(!contents.mMine);
     HeaderFieldValue *hfv = new HeaderFieldValue(contents.data(),  
contents.size());

+   if(contentType.subType()=="sipfrag"||contentType.subType() 
=="external-body")
+   {
+      // !bwc! The parser for sipfrag requires padding at the end of  
the hfv.
+      HeaderFieldValue* temp = hfv;
+      hfv = new HeaderFieldValue(*temp,HeaderFieldValue::CopyPadding);
+      delete temp;
+   }
+
     Contents* c;
     if (ContentsFactoryBase::getFactoryMap().find(contentType) !=  
ContentsFactoryBase::getFactoryMap().end())
     {

Best regards,
Byron Campen

> Hmm, probably not; looking at MultipartMixedContents we probably  
> need a fairly expensive special case copy when the content-type is  
> message/sipfrag or message/external.  It should be easy to  
> reproduce&fix.
>
>
>
> -Derek
>
>
>
> From: Byron Campen [mailto:bcampen at estacado.net]
> Sent: Thursday, August 31, 2006 11:54 AM
> To: Derek MacDonald
> Cc: 'Kath, Heiner'; resiprocate-devel at list.sipfoundry.org
> Subject: Re: [reSIProcate] bug in SipFrag.cxx
>
>
>
>           Yes, but is this code safe when you have a sipfrag in a  
> multipart-mixed payload?
>
>
>
> Best regards,
>
> Byron Campen
>
>
>
>
> Yes; the transport classes use MsgHeaderScanner::allocateBuffer(int  
> size) to guarantee that there are 5 extra bytes at the end of each  
> buffer. I do not know of any memory problems as a result of this  
> with the reciprocate transports.
>
>
>
> -Derek
>
>
>
> From: resiprocate-devel-bounces at list.sipfoundry.org  
> [mailto:resiprocate-devel-bounces at list.sipfoundry.org] On Behalf Of  
> Byron Campen
> Sent: Thursday, August 31, 2006 10:34 AM
> To: Kath, Heiner
> Cc: resiprocate-devel at list.sipfoundry.org
> Subject: Re: [reSIProcate] bug in SipFrag.cxx
>
>
>
>           Good eye. Your solution sounds about as optimal as can be  
> managed.
>
>
>
> Best regards,
>
> Byron Campen
>
>
>
> Hi,
>
>
>
> I found this in SipFrag.cxx:
>
>
>
> {
>
>
>
>
>
>    const char *constBuffer = pb.position();
>
>
>
>
>
>    // than size bytes of the message.
>
>
>
>    msgHeaderScanner.prepareForFrag(mMessage, hasStartLine(buffer,  
> size));
>
>    char saveTermCharArray[sentinelLength];
>
>    saveTermCharArray[0] = termCharArray[0];
>
>    saveTermCharArray[2] = termCharArray[2];
>
>    termCharArray[0] = '\r';
>
>    termCharArray[2] = '\r';
>
>    char *scanTermCharPtr;
>
>        msgHeaderScanner.scanChunk(buffer,
>
>                                   &scanTermCharPtr);
>
>    termCharArray[1] = saveTermCharArray[1];
>
>    termCharArray[3] = saveTermCharArray[3];
>
>
>
> The problem with this code is that the sentinel is wrote *behind*  
> the ParseBuffer. If the ParseBuffer stands at the very end of an  
> allocated buffer, this code writes 4 bytes behind it. So it  
> happened in our application. As a consequence a further allocation  
> of memory inside of scanChunk() failed – probably because some  
> administration information of the memory heap was overwritten.
>
> I can easily reproduce this using the gflags tool of the Mircosoft  
> debugging tools.
>
> My solution consists in allocating a new buffer that has the  
> required size (+4), copying the content of the ParseBuffer to it.  
> The new buffer is referenced by the
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20060911/8e0ba1d0/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2369 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20060911/8e0ba1d0/attachment.bin>