[reSIProcate] Bug in Data::append()

[david Butcher]

We went to some effort to make sure the NULL is there to aid in debugging.

david

On 3/22/06, Brian Cuthie <brian at systemix.com> wrote:
>
>
> It seems to me (and I'm admittedly a ReSIProcate neophyte) that the
> correct thing is to remove the statement in append() that sets the NULL.
> If the buffer needed a NULL, then it would be there already. And it
> looks to me like anything that wants a NULL accesses the string through
> c_str() anyway, which explicitly adds one.
>
> FWIW: I remove the statement in append() and I haven't seen any problem
> yet.
>
> -brian
>
>
> > -----Original Message-----
> > From: Scott Godin [mailto:slgodin at icescape.com]
> > Sent: Wednesday, March 22, 2006 5:52 PM
> > To: Byron Campen; Brian Cuthie
> > Cc: resiprocate-devel at list.sipfoundry.org
> > Subject: 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 at list.sipfoundry.org
> > [mailto:resiprocate-devel-bounces at list.sipfoundry.org] On
> > Behalf Of Byron Campen
> > 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
> >
> >
> _______________________________________________
> 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/20060324/ecd1af97/attachment.htm>