[reSIProcate] memory leak bug in TransactionState

[Adam Roach]

And, to be clear, even with asserts turned off, these codepaths appear
to cause problems. Using about 60 lines of perl, I was able to take a
popular, widely-deployed resip-based application and performed a remote
attack that drove its memory footprint from about 20 megs to 200 megs in
less than a minute. Stop the attack, and the bloat remains.

Stack enforcement of request method/cseq-method matching seems to be a
perfectly reasonable solution; send out 400 errors if things don't match.

/a

Byron Campen wrote:
> Ok, here is bug #3 (this is a corollary of bug #2). If an attacker 
> sends us an INVITE with CANCEL in the CSeq, the TU's response hits 
> this code:
>
> *snip*
>    else if (isResponse(msg, 100, 699) && isFromTU(msg))
>    {
>       SipMessage* sip = dynamic_cast<SipMessage*>(msg);
>       int code = sip->header(h_StatusLine).responseCode();
> *      switch (sip->header(h_CSeq).method())*
>       {
>
> //...
>
>          case CANCEL:
>             *assert(0);*
>             break;
>
>             
>
> *snip*
>
> It seems to me that the stack is being liberal for stuff coming off 
> the wire, and strict for stuff coming from the TU. This becomes 
> problematic because we are expecting the TU to fix garbage  that the 
> stack hands it. We should not be expecting the TU to do 
> protocol-repair on stuff that we, the stack, will need to use.
>
> I propose that we ensure that CSeq is well-formed, and matches the 
> RequestLine (if any), for every message that comes off the wire. If 
> the stack is going to be making use of it, it is the stack's 
> responsibility to ensure that it is usable.
>
> Best regards,
> Byron Campen
>
>> Alright, suppose something sends us an INVITE with a non-matching, 
>> unknown method in CSeq. This will parse successfully, since it is 
>> well-formed. This INVITE will be sent up to the TU, which will reply 
>> (hopefully). Unfortunately, TransactionState ingores the response...
>>
>> *snip*
>>    else if (isResponse(msg, 100, 699) && isFromTU(msg))
>>    {
>>       SipMessage* sip = dynamic_cast<SipMessage*>(msg);
>>       int code = sip->header(h_StatusLine).responseCode();
>>       switch (sip->header(h_CSeq).method())
>>       {
>> //...
>>
>>
>>          *default*:
>>             //StackLog (<< "Received response to non invite or 
>> cancel. Ignoring");
>>             delete msg;
>>             break;
>>
>> *snip*
>>
>>
>> So, should the stack be verifying that the method in CSeq matches the 
>> request-line when a request comes in? This is the second bug I have 
>> found in the last hour or so that could be solved by adding this check. 
>>
>> Best regards,
>> Byron Campen
>>
>> _______________________________________________
>> resiprocate-devel mailing list
>> resiprocate-devel at list.sipfoundry.org 
>> <mailto:resiprocate-devel at list.sipfoundry.org>
>> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at list.sipfoundry.org
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel