[reSIProcate] bug in SipFrag.cxx

[Kath, Heiner]

Hi,

 

Thanks for your comments.

 

With the extra bytes allocated in MsgHeaderScanner::allocateBuffer(int
size)    I wonder to get the access violations using the memory checker
tool. It looks like the problem is somewhere else. I definitely see the
access violation at 

   saveTermCharArray[2] = termCharArray[2];

That makes 2 missing bytes in the allocated buffer.

 

Heiner

 

________________________________

From: Derek MacDonald [mailto:derek at counterpath.com] 
Sent: Freitag, 1. September 2006 01:50
To: 'Byron Campen'
Cc: Kath, Heiner; resiprocate-devel at list.sipfoundry.org
Subject: RE: [reSIProcate] bug in SipFrag.cxx

 

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/20060901/b46287be/attachment.htm>