RE: [reSIProcate] Bug in Data::append()
Then why does c_str() need to allocate more storage for the NULL?
-brian
> -----Original Message-----
> From: Byron Campen [mailto:bcampen@xxxxxxxxxxxx]
> Sent: Wednesday, March 22, 2006 5:59 PM
> To: Scott Godin
> Cc: Brian Cuthie; resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [reSIProcate] Bug in Data::append()
>
> I don't think mCapacity includes that last byte of room
> either. I think that last byte is reserved specifically for
> that null, and strictly off-limits for everything else. (Take
> a look at every "new"
> in Data.cxx, they all have that extra byte of room)
>
> Best regards,
> Byron
>
> > 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
> >
>
>