Possible misassumption in HandleFirstSignallingChannelPDU
In our software that we mentioned a few days ago, in addition to the lockups, we observed a problem when we apparently received an incoming call, but CreateConnection is called with a valid PDU that is not a Setup Message one.
It happens when the incoming call comes from a Cisco gateway, and the call hangs up at a point where the gateway has sent the Admission Request to the gatekeeper, receives its corresponding admission confirm, but has not sent yet the setup message to our software / destination endpoint.
In these circumstances we saw that the gateway has opened the signaling socket with our endpoint, but the first packet it sends is a Release Complete because the call was hung before sending the setup.
PDU trace from our Gatekeeper:
2017-05-27 14:46:28.853 5d37 INFO pdu.call.a.ARQ.recv : Received from ip$x.x.x.x:54432. 2017-05-27 14:46:28.855 5d37 INFO pdu.call.a.ACF.send : Sending to ip$x.x.x.x:54432. 2017-05-27 14:46:28.865 5d37 INFO pdu.call.d.DRQ.recv : Received from ip$x.x.x.x:54432. 2017-05-27 14:46:28.866 5d37 INFO pdu.call.d.DCF.send : Sending to ip$x.x.x.x:54432.
See the time-stamps, showing that the ACF and DRQ are closely followed. We receive the call at our endpoint, in the CreateConnection function at the same time point as the DRQ / DCF.
In our software, in some cases we return NULL if we cannot handle the incoming connection (not enough setup data, e.g.), and if we are running an executable compiled in Debug mode, an assert fails on the following operator:
H225_H323_UU_PDU_h323_message_body :: operator const H225_Setup_UUIE & () const #endif { #ifndef PASN_LEANANDMEAN PAssert (PIsDescendant (PAssertNULL (choice), H225_Setup_UUIE), PInvalidCast); #endif Return * (H225_Setup_UUIE *) choice; }
It is caused by the following line (transports.cxx: 1126):
H225_Setup_UUIE & setup = pdu.m_h323_uu_pdu.m_h323_message_body;
This code assumes that the initial PDU is a Setup one without checking it, when it may not necessarily be. That can cause a DoS in a software that has core dump enabled when an assert occurs, or if there is some code in HandleFirstSignallingChannelPDU / OnIncomingConnection and sub-calls that trust and handles the PDU object assuming it is a Setup message.
I've made the following patch on the HandleFirstSignallingChannelPDU function, which verifies that the PDU is a Setup message, otherwise it returns FALSE (same behavior as if it had not been able to read a valid q931 pdu), since we think it does not make much sense to call OnIncomingConnection if the initial PDU is not a Setup one:
--------------------------------------------------------------------------------
--- src/transports.cxx.orig 2015-11-24 18:51:06.385755223 +0100 +++ src/transports.cxx 2017-05-29 11:15:20.777411444 +0200 @@ -1106,7 +1106,13 @@ return FALSE; }
- unsigned callReference = pdu.GetQ931().GetCallReference(); + const Q931 &firstQ931PDU = pdu.GetQ931(); + if (firstQ931PDU.GetMessageType() != Q931::SetupMsg) { + PTRACE(1, "H225\tFirst pdu is not a setup one, connection not started."); + return FALSE; + } + + unsigned callReference = firstQ931PDU.GetCallReference(); PTRACE(3, "H225\tIncoming call, first PDU: callReference=" << callReference);
// Get a new (or old) connection from the endpoint
--------------------------------------------------------------------------------
A place where we could also put the patch is in the HandleSignallingSocket function, which apparently is only used from HandleFirstSignallingChannelPDU, but as it has a more generic name and may give rise to doubts of its behavior.
Regards,
Jose Uceda
Hi Jose,
thanks for finding that bug!
I have committed your patch with minor changes to the CVS.
Regards, Jan
participants (2)
-
Jan Willamowius
-
Jose Uceda