[reSIProcate] SipFrag parsing leads to heap corruption
Alan Hawrylyshen
alan at jasomi.com
Tue Nov 8 09:15:09 CST 2005
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
More information about the resiprocate-devel
mailing list