RE: [reSIProcate] SipFrag parsing leads to heap corruption
I just checked in a fix to this. SipMessage will maintain padding during
copy construction. I also cleanup up the transport code to use an
allocation routine from MsgHeaderScanner rather than manually adding the
padded bytes.
Good find Sasha!
Thanks,
Derek
CONFIDENTIALITY NOTICE
This email and any files transmitted with it contains proprietary
information and, unless expressly stated otherwise, all contents and
attachments are confidential. This email is intended for the addressee(s)
only and access by anyone else is unauthorized. If you are not an addressee,
any disclosure, distribution, printing or copying of the contents of this
email or its attachments, or any action taken in reliance on it, is
unauthorized and may be unlawful. If you are not an addressee, please inform
the sender immediately and then delete this email and any copies of it.
Thank you for your co-operation.
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx [mailto:resiprocate-
> devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Alan Hawrylyshen
> Sent: Tuesday, November 08, 2005 7:15 AM
> To: Sasha Youkhananov
> Cc: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [reSIProcate] SipFrag parsing leads to heap corruption
>
>
> On 8-Nov-05, at 00:42 , Sasha Youkhananov wrote:
>
> > Hello resiprocate developers.
> >
> >
> >
> > I build a soft phone application under Windows XP [snip]
> >
> > I examined the situation several times and found that SipFrag::parse
> > () call leads to memory corruption.
> >
> > Here is the call stack:
> >
> > resip::DialogUsageManager::process ()
> >
> > resip::DialogUsageManager::internalProcess ()
> >
> > resip::DialogUsageManager::processRequest ()
> >
> > [snip]
> > resip::MsgHeaderScanner::scanChunk ()
> >
> > resip::processMsgHeaderStatusLine ()
> >
> > resip::SipMessage::setStartLine
> >
> >
> >
> > So, during construction of BaseSubscription the original request
> > with sip-frag body is copied into mLastRequest member. When copy
> > constructor of SipMessage works it copies mContentsHfv of original
> > request to mContentsHfv of message that is being constructed. The
> > copy constructor of HeaderFieldValue copies exactly mFieldLength
> > bytes to constructed object.
> >
> > When SipFrag parser starts its processing it writes 4 consecutive
> > bytes (according to its algorithm) in area that is located at the
> > address: buffer + size. That address is behind the allocated memory
> > (was previously allocated in HeaderFieldValue copy constructor) and
> > in debug mode is used to store the internal heap data (4
> > consecutive 0xFD). Thus, the next memory allocation request in
> > SipMessage::setStartLine fails.
> >
> >
> >
> > In order to repair this situation I changed copy constructor of
> > HeaderFieldValue type to following:
> >
> >
> >
> > HeaderFieldValue::HeaderFieldValue (const HeaderFieldValue& hfv)
> >
> > : mField(0),
> >
> > mFieldLength(hfv.mFieldLength),
> >
> > mMine(true)
> >
> > {
> >
> > //InfoLog (<< "Making a copy of a HFV" << hex << this);
> >
> > mField = new char[mFieldLength + 5];
> >
> > memcpy(const_cast<char*>(mField), hfv.mField, mFieldLength + 5);
> >
> > }
> >
> >
> >
> > Of course, this change will affect other stack parts which use this
> > type, but I haven't found more appropriate place to my fix.
> >
> > I'm sure you can suggest the better way to repair it.
> >
> > Wait for any answer about this problem.
>
> Sasha;
>
> Thanks for the excellent report on this. I think it's high time we
> fixed the MHS. There have been bits and pieces scattered around the
> code to deal with the 4 byte over-run. I think it's not appropriate
> and the MHS should scale back it's aggression, I welcome anyone to
> argue in the other direction, but you'll have to convince me that
> it's worthwhile to do a fix anywhere but in the MHS itself.
>
> Thanks,
>
> Alan
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel