Re: [reSIProcate] bug in SipFrag.cxx

[Byron Campen <bcampen@xxxxxxxxxxxx>]

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@xxxxxxxxxxxx]
Sent: Thursday, August 31, 2006 11:54 AM
To: Derek MacDonald
Cc: 'Kath, Heiner'; resiprocate-devel@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Byron Campen
Sent: Thursday, August 31, 2006 10:34 AM
To: Kath, Heiner
Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
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

Attachment: smime.p7s
Description: S/MIME cryptographic signature