RE: [reSIProcate] Bug in Data::append()
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@xxxxxxxxxxxx]
> Sent: Wednesday, March 22, 2006 5:52 PM
> To: Byron Campen; Brian Cuthie
> Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> 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@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
>
>