reSIProcate

Mailing Lists

Source Code

Google Search:



< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] Bug in Data::append()

I don't think mCapacity includes that last byte of room either. I think that last byte is reserved specifically for that null, and strictly off-limits for everything else. (Take a look at every "new" in Data.cxx, they all have that extra byte of room) 

Best regards,
Byron


Actually it looks like there is a problem if mCapacity == mSize + len
and mMime != Share. In this case by then end of the function mCapacity 
== mSize and mBuf[mSize] is out of range.

Perhaps the initial if statement should read:
if (mCapacity <= mSize + len)

Scott

-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of
Byron Campen
Sent: Wednesday, March 22, 2006 5:45 PM
To: Brian Cuthie
Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Bug in Data::append()

        Actually, that buffer has size mSize+1, so that null termination
is
not causing memory corruption. But, your heap-corruption issue is
still real, so I ask what version of resiprocate you are using.

Best regards,
Byron Campen




Hello,

I'm new to this list, and to using the Reciprocate stack. However, I
think I found a bug and wanted to hear what people think. I'm sure I'm 
doing this wrong, so please don't be shy about helping me
understand the
correct procedure.

Background:

While testing the stack under Windows, I had problems with a heap
corruption when writing debug log messages using the VSDebugWindow log 
type.

BUG:

The following code in Data::append() appears to be in error. The
NULL is
written past the end of the buffer, and furthermore I think it is
unnecessary, since c_str() adds a NULL.

Data&
Data::append(const char* str, size_type len)
{
   assert(str);
   if (mCapacity < mSize + len)
   {
      // .dlb. pad for future growth?
      resize(((mSize + len +16)*3)/2, true);
   }
   else
   {
      if (mMine == Share)
      {
         char *oldBuf = mBuf;
         mCapacity = mSize + len;
         mBuf = new char[mSize + len];
         memcpy(mBuf, oldBuf, mSize);
         mMine = Take;
      }
   }

   // could conceivably overlap
   memmove(mBuf + mSize, str, len);
   mSize += len;
   mBuf[mSize] = 0;   // ERROR

   return *this;
}

Finally, there appears to be a related bug fix to Data::c_str()
(#5408).
I'm wondering if someone was tracking this same problem and tried
to fix
it there, since resize() will copy the buffer anyway, making the check 
in c_str() unecessary.

Cheers,

Brian

_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel

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