[Asterisk-code-review] AST-2017-014: res pjsip - Missing contact header can cause c... (asterisk[13])

[Kevin Harwell]

Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/7720 )

Change subject: AST-2017-014: res_pjsip - Missing contact header can cause crash
......................................................................

AST-2017-014: res_pjsip - Missing contact header can cause crash

Those SIP messages that create dialogs require a contact header to be present.
If the contact header was missing from the message it could cause Asterisk to
crash.

This patch checks to make sure SIP messages that create a dialog contain the
contact header. If the message does not and it is required Asterisk now returns
a "400 Missing Contact header" response. Also added NULL checks when retrieving
the contact header that were missing as a "just in case".

ASTERISK-27480 #close

Change-Id: I1810db87683fc637a9e3e1384a746037fec20afe
---
M res/res_pjsip.c
M res/res_pjsip/pjsip_message_filter.c
M res/res_pjsip_pubsub.c
3 files changed, 22 insertions(+), 6 deletions(-)

Approvals:
  Jenkins2: Verified
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 1fce19b..392f9a6 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3241,7 +3241,7 @@
 	ast_assert(status != NULL);
 
 	contact_hdr = pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, NULL);
-	if (ast_sip_set_tpselector_from_ep_or_uri(endpoint, pjsip_uri_get_uri(contact_hdr->uri),
+	if (!contact_hdr || ast_sip_set_tpselector_from_ep_or_uri(endpoint, pjsip_uri_get_uri(contact_hdr->uri),
 		&selector)) {
 		return NULL;
 	}
diff --git a/res/res_pjsip/pjsip_message_filter.c b/res/res_pjsip/pjsip_message_filter.c
index 085d978..427aec7 100644
--- a/res/res_pjsip/pjsip_message_filter.c
+++ b/res/res_pjsip/pjsip_message_filter.c
@@ -429,15 +429,27 @@
 		return PJ_TRUE;
 	}
 
-	while ((contact =
-		(pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT,
-			contact ? contact->next : NULL))) {
+
+	contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(
+		rdata->msg_info.msg, PJSIP_H_CONTACT, NULL);
+
+	if (!contact && pjsip_method_creates_dialog(&rdata->msg_info.msg->line.req.method)) {
+		/* A contact header is required for dialog creating methods */
+		static const pj_str_t missing_contact = { "Missing Contact header", 22 };
+		pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata, 400,
+				&missing_contact, NULL, NULL);
+		return PJ_TRUE;
+	}
+
+	while (contact) {
 		if (!contact->star && !is_sip_uri(contact->uri)) {
 			print_uri_debug(URI_TYPE_CONTACT, rdata, (pjsip_hdr *)contact);
 			pjsip_endpt_respond_stateless(ast_sip_get_pjsip_endpoint(), rdata,
 				PJSIP_SC_UNSUPPORTED_URI_SCHEME, NULL, NULL, NULL);
 			return PJ_TRUE;
 		}
+		contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(
+			rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next);
 	}
 
 	return PJ_FALSE;
diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index d921aa3..88005b8 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -613,8 +613,12 @@
 		expires = expires_hdr ? expires_hdr->ivalue : DEFAULT_PUBLISH_EXPIRES;
 		sub_tree->persistence->expires = ast_tvadd(ast_tvnow(), ast_samp2tv(expires, 1));
 
-		pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, contact_hdr->uri,
-			sub_tree->persistence->contact_uri, sizeof(sub_tree->persistence->contact_uri));
+		if (contact_hdr) {
+			pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, contact_hdr->uri,
+					sub_tree->persistence->contact_uri, sizeof(sub_tree->persistence->contact_uri));
+		} else {
+			ast_log(LOG_WARNING, "Contact not updated due to missing contact header\n");
+		}
 
 		/* When receiving a packet on an streaming transport, it's possible to receive more than one SIP
 		 * message at a time into the rdata->pkt_info.packet buffer. However, the rdata->msg_info.msg_buf

-- 
To view, visit https://gerrit.asterisk.org/7720
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I1810db87683fc637a9e3e1384a746037fec20afe
Gerrit-Change-Number: 7720
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171222/7cf8e787/attachment.html>