[reSIProcate] Bug in Data::append()

[Byron Campen]

	Ah, I was looking at the code on head. That bug was probably fixed a  
while ago. There have been a few (that I know of) nasty memory  
corruption bugs that have been fixed since that release. I will echo  
the chorus of "please update to head" that we've been hearing a lot  
these days.

Best regards,
Byron Campen

>
> Byron,
>
> Are you sure? Looks to me like the buffer is mSize in length.  
> Here's the
> relevant parts:
>
> mBuf = new char[mSize + len];
> mSize += len;
> mBuf[mSize] = 0; // Buffer overflow
>
> I'm using the 0.8.0 tarball.
>
> -brian
>
>
>> -----Original Message-----
>> From: Byron Campen [mailto:bcampen at estacado.net]
>> Sent: Wednesday, March 22, 2006 5:45 PM
>> To: Brian Cuthie
>> Cc: resiprocate-devel at list.sipfoundry.org
>> 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 at list.sipfoundry.org
>>> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>>
>>

-------------- 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/20060322/f51bc183/attachment.bin>