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

[Byron Campen <bcampen@xxxxxxxxxxxx>]

	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@xxxxxxxxxxxx]
> > 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