[reSIProcate] bug in SipFrag.cxx

[Derek MacDonald]

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:

 

void 

SipFrag::parse(ParseBuffer& pb)

{

//   DebugLog(<< "SipFrag::parse: " << pb.position());

 

   mMessage = new SipMessage();

 

   pb.assertNotEof();

   const char *constBuffer = pb.position();

   char *buffer = const_cast<char *>(constBuffer);

 

   size_t size = pb.end() - pb.position();

 

   // !ah! removed size check .. process() cannot process more

   // than size bytes of the message.

 

 

   MsgHeaderScanner msgHeaderScanner;

   msgHeaderScanner.prepareForFrag(mMessage, hasStartLine(buffer, size));

   enum { sentinelLength = 4 };  // Two carriage return / line feed pairs.

   char saveTermCharArray[sentinelLength];

   char *termCharArray = buffer + size;

   saveTermCharArray[0] = termCharArray[0];

   saveTermCharArray[1] = termCharArray[1];

   saveTermCharArray[2] = termCharArray[2];

   saveTermCharArray[3] = termCharArray[3];

   termCharArray[0] = '\r';

   termCharArray[1] = '\n';

   termCharArray[2] = '\r';

   termCharArray[3] = '\n';

   char *scanTermCharPtr;

   MsgHeaderScanner::ScanChunkResult scanChunkResult =

       msgHeaderScanner.scanChunk(buffer,

                                  size + sentinelLength,

                                  &scanTermCharPtr);

   termCharArray[0] = saveTermCharArray[0];

   termCharArray[1] = saveTermCharArray[1];

   termCharArray[2] = saveTermCharArray[2];

   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 SipFrag object and is freed on destruction of the latter.
It is up to the resip-pro's to find a smarter solution.

 

I like resip :-)

 

Heiner

 

 

 

 

 

_______________________________________________

resiprocate-devel mailing list

resiprocate-devel at list.sipfoundry.org

https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20060831/ddca0be6/attachment.htm>