< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] memory leak bug in TransactionState


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

Attachment: smime.p7s
Description: S/MIME cryptographic signature