Re: [reSIProcate] resiprocate crash due to no free memory when receiving sip message with large header
Responses inline, since this is kinda complicated:
Hi,
I have run a test tool that tries to find faults in sip user agents by sending faulty or weird messages.
The problem I have found is that when resiprocate gets a message via TCP with an extremely large header it will use up all memory and then crash from a "new" that fails.
If I understand ConnectionBase::preparseNewBytes and MsgHeaderScanner::scanChunk correctly it will allocate a new buffer of size 2048 or greater. If a header does not fit in this it will leave some bytes unparsed and parse them next time. These unparsed bytes will then be copied into the next buffer that is bigger than the previous one. Then the buffer is added to the SipMessage that takes over the ownership of the buffer.
The second buffer will then again not be big enough and the procedure is repeated. This will lead to increasingly larger buffers ,containing more and more of the large header, being allocated and given to the SipMessage.
It looks like we are taking things in smallish chunks in order to save room; if we grab a chunk that is 50K long, and it contains 50 SIP messages, that 50K buffer will be added to the first SipMessage we parse, and then will be copied and trimmed. The next SipMessage will get a 49K buffer added. And the next a 48K buffer. It would be difficult to just give the SipMessage a memcpy of the part of the buffer it needs, since that would destroy the overlay that the MsgHeaderScanner just put together (ie, all those pointers would have to be adjusted to point into the new buffer). So, the buffers are kept relatively small. I am not sure how you're seeing "increasingly large buffers" being added; they should be kept to a maximum of 2K.
In the failing case the header is >1MB, so there will be approximately 500 buffers in the SipMessage containing part of the large header.
This is baffling; 1M is not a large amount of memory. Are you sure it is the buffers that are pushing us over the edge?
What I want to know is why the buffers are given to the SipMessage in this way. I could not even find a place where the buffers stored in SipMessage was actually used.
The buffers are used; as the pre-parser runs, it passes pointers and lengths to SipMessage that point into these buffers. The buffers are kept around to keep the memory intact, so the pointers aren't left dangling. What I did to "fix" this is I check if chunkLength (buffer size) is bigger than 50kb. If so, the header is too big and the connection is dropped.
Someone with more in-depth knowledge of how the pre-parser works and what it is meant to do, should probably fix the memory leak properly though. If a header is just big enough so that the last allocation succeeds, the SipMessage would end up owning buffers that has allocated all your memory and something else could crash from a failed allocation.
Here is a diff with my addition to drop the connection.
ConnectionBase.cxx.diff:
143a144,154
> if (chunkLength > 50000)
> {
> WarningLog(<< "Discarding preparse! Too big Header");
> delete [] mBuffer;
> mBuffer = 0;
> delete mMessage;
> mMessage = 0;
> //.jacob. Shouldn't the state also be set here?
> delete this;
> return;
> }
Since a chunk can contain more than one SIP message, we can't really arbitrarily restrict the chunk-size, although we _can_ put code in that will notice when we have run the pre-parser over an unreasonably large number of bytes.
Best regards, Byron Campen
|
Attachment:
smime.p7s
Description: S/MIME cryptographic signature