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

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


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