RE: [reSIProcate] Bug in Data::append()
Ok, thanks Byron.
Is head pretty stable, then?
-brian
> -----Original Message-----
> From: Byron Campen [mailto:bcampen@xxxxxxxxxxxx]
> Sent: Wednesday, March 22, 2006 6:05 PM
> To: Brian Cuthie
> Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [reSIProcate] Bug in Data::append()
>
> 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
> >>
> >>
>
>