Re: [reSIProcate] assert bug in TransactionState
I would recommend putting the check in Transport::basicCheck to ensure
that any request coming in has a consistent method in the Request-Line
to the method in the CSeq. If it does not, we should reject with a 400
right from the Transport.
On 8/24/06, Byron Campen <bcampen@xxxxxxxxxxxx> wrote:
When a sip message comes off the state-machine fifo, we check to see whether
it is for a CANCEL transaction, like so...
*snip*
// This ensures that CANCEL requests form unique transactions
if (sip && sip->header(h_CSeq).method() == CANCEL)
{
tid += "cancel";
}
TransactionState* state = 0;
if (message->isClientTransaction()) state =
controller.mClientTransactionMap.find(tid);
else state = controller.mServerTransactionMap.find(tid);
*snip*
It is important to note that we are using the CSeq for both requests and
responses here. However, if we look at the code for request-handling in
processServerInvite, we see the following...
*snip*
if (isRequest(msg) && isFromWire(msg))
{
SipMessage* sip = dynamic_cast<SipMessage*>(msg);
switch (sip->header(h_RequestLine).getMethod())
{
//...
case CANCEL:
assert(0);
break;
*snip*
So, if someone sent us a CANCEL with the CSeq deliberately malformed to read
something like
"CSeq: INVITE 1", we would not identify this as a CANCEL in the first block
of code, and send this request to the original INVITE server transaction for
processing. Then, the second block would cause us to assert, because it
would see CANCEL in the request-line.
I propose we write SipMessage::method(), which will check the Request line
if it is a request, and the CSeq if it is a response. This would be the
"correct" way to check the method of a SipMessage.
Best regards,
Byron Campen
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxxxxxx
https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel