[Asterisk-code-review] AST-2016-004: Fix crash on REGISTER with long URI. (asterisk[master])

Mark Michelson asteriskteam at digium.com
Thu Apr 14 11:28:33 CDT 2016


Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/2597

Change subject: AST-2016-004: Fix crash on REGISTER with long URI.
......................................................................

AST-2016-004: Fix crash on REGISTER with long URI.

Due to some ignored return values, Asterisk could crash if processing an
incoming REGISTER whose contact URI was above a certain length.

ASTERISK-25707 #close
Reported by George Joseph

Patches:
    0001-res_pjsip-Validate-that-URIs-don-t-exceed-pjproject-.patch

AST-2016-004

Change-Id: I3ea7cee16f29c8088794de3085ca7523c1c4833d
---
M res/res_pjsip/include/res_pjsip_private.h
M res/res_pjsip/location.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_registrar.c
4 files changed, 93 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/97/2597/1

diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h
index 2cc9fea..24b8edf 100644
--- a/res/res_pjsip/include/res_pjsip_private.h
+++ b/res/res_pjsip/include/res_pjsip_private.h
@@ -306,4 +306,11 @@
  * \brief Finds or creates contact_status for a contact
  */
 struct ast_sip_contact_status *ast_res_pjsip_find_or_create_contact_status(const struct ast_sip_contact *contact);
+
+/*!
+ * \internal
+ * \brief Validate that the uri meets pjproject length restrictions
+ */
+int ast_sip_validate_uri_length(const char *uri);
+
 #endif /* RES_PJSIP_PRIVATE_H_ */
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index bc14f18..0a82f3a 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -29,6 +29,11 @@
 #include "asterisk/statsd.h"
 #include "asterisk/named_locks.h"
 
+#include "asterisk/res_pjproject.h"
+
+static int pj_max_hostname = PJ_MAX_HOSTNAME;
+static int pjsip_max_url_size = PJSIP_MAX_URL_SIZE;
+
 /*! \brief Destructor for AOR */
 static void aor_destroy(void *obj)
 {
@@ -405,6 +410,43 @@
 	return cmp;
 }
 
+int ast_sip_validate_uri_length(const char *contact_uri)
+{
+	pjsip_uri *uri;
+	pjsip_sip_uri *sip_uri;
+	pj_pool_t *pool;
+	int max_length = pj_max_hostname - 1;
+
+	if (strlen(contact_uri) > pjsip_max_url_size - 1) {
+		return -1;
+	}
+
+	if (!(pool = pjsip_endpt_create_pool(ast_sip_get_pjsip_endpoint(), "uri validation", 512, 512))) {
+		ast_log(LOG_ERROR, "Unable to allocate pool for uri validation\n");
+		return -1;
+	}
+
+	if (!(uri = pjsip_parse_uri(pool, (char *)contact_uri, strlen(contact_uri), 0)) ||
+	    (!PJSIP_URI_SCHEME_IS_SIP(uri) && !PJSIP_URI_SCHEME_IS_SIPS(uri))) {
+		pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+		return -1;
+	}
+
+	sip_uri = pjsip_uri_get_uri(uri);
+	if (sip_uri->port == 0) {
+		max_length -= strlen("_sips.tcp.");
+	}
+
+	if (sip_uri->host.slen > max_length) {
+		pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+		return -1;
+	}
+
+	pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), pool);
+
+	return 0;
+}
+
 /*! \brief Custom handler for permanent URIs */
 static int permanent_uri_handler(const struct aco_option *opt, struct ast_variable *var, void *obj)
 {
@@ -426,6 +468,11 @@
 
 		if (ast_strlen_zero(contact_uri)) {
 			continue;
+		}
+
+		if (ast_sip_validate_uri_length(contact_uri)) {
+			ast_log(LOG_ERROR, "Contact uri or hostname length exceeds pjproject limit: %s\n", contact_uri);
+			return -1;
 		}
 
 		if (!aor->permanent_contacts) {
@@ -1015,6 +1062,10 @@
 	struct ast_sorcery *sorcery = ast_sip_get_sorcery();
 	int i;
 
+	ast_pjproject_get_buildopt("PJ_MAX_HOSTNAME", "%d", &pj_max_hostname);
+	/* As of pjproject 2.4.5, PJSIP_MAX_URL_SIZE isn't exposed yet but we try anyway. */
+	ast_pjproject_get_buildopt("PJSIP_MAX_URL_SIZE", "%d", &pjsip_max_url_size);
+
 	ast_sorcery_apply_default(sorcery, "contact", "astdb", "registrar");
 	ast_sorcery_apply_default(sorcery, "aor", "config", "pjsip.conf,criteria=type=aor");
 
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index dd69ff2..f85996e 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -1290,10 +1290,18 @@
 		ast_log(LOG_ERROR, "No server URI specified on outbound registration '%s'\n",
 			ast_sorcery_object_get_id(applied));
 		return -1;
+	} else if (ast_sip_validate_uri_length(applied->server_uri)) {
+			ast_log(LOG_ERROR, "Server URI or hostname length exceeds pjpropject limit '%s'\n",
+				ast_sorcery_object_get_id(applied));
+			return -1;
 	} else if (ast_strlen_zero(applied->client_uri)) {
 		ast_log(LOG_ERROR, "No client URI specified on outbound registration '%s'\n",
 			ast_sorcery_object_get_id(applied));
 		return -1;
+	} else if (ast_sip_validate_uri_length(applied->client_uri)) {
+			ast_log(LOG_ERROR, "Client URI or hostname length exceeds pjpropject limit '%s'\n",
+				ast_sorcery_object_get_id(applied));
+			return -1;
 	} else if (applied->line && ast_strlen_zero(applied->endpoint)) {
 		ast_log(LOG_ERROR, "Line support has been enabled on outbound registration '%s' without providing an endpoint\n",
 			ast_sorcery_object_get_id(applied));
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 97cf34a..a94babd 100644
--- a/res/res_pjsip_registrar.c
+++ b/res/res_pjsip_registrar.c
@@ -18,6 +18,7 @@
 
 /*** MODULEINFO
 	<depend>pjproject</depend>
+	<depend>res_pjproject</depend>
 	<depend>res_pjsip</depend>
 	<support_level>core</support_level>
  ***/
@@ -33,6 +34,7 @@
 #include "asterisk/taskprocessor.h"
 #include "asterisk/manager.h"
 #include "asterisk/named_locks.h"
+#include "asterisk/res_pjproject.h"
 #include "res_pjsip/include/res_pjsip_private.h"
 
 /*** DOCUMENTATION
@@ -51,6 +53,9 @@
 		</description>
 	</manager>
  ***/
+
+static int pj_max_hostname = PJ_MAX_HOSTNAME;
+static int pjsip_max_url_size = PJSIP_MAX_URL_SIZE;
 
 /*! \brief Internal function which returns the expiration time for a contact */
 static int registrar_get_expiration(const struct ast_sip_aor *aor, const pjsip_contact_hdr *contact, const pjsip_rx_data *rdata)
@@ -86,7 +91,7 @@
 	/*! \brief Pool used for parsing URI */
 	pj_pool_t *pool;
 	/*! \brief URI being looked for */
-	pjsip_uri *uri;
+	pjsip_sip_uri *uri;
 };
 
 /*! \brief Callback function for finding a contact */
@@ -114,6 +119,7 @@
 	while ((contact = (pjsip_contact_hdr *) pjsip_msg_find_hdr(rdata->msg_info.msg, PJSIP_H_CONTACT, contact->next))) {
 		int expiration = registrar_get_expiration(aor, contact, rdata);
 		RAII_VAR(struct ast_sip_contact *, existing, NULL, ao2_cleanup);
+		char contact_uri[pjsip_max_url_size];
 
 		if (contact->star) {
 			/* The expiration MUST be 0 when a '*' contact is used and there must be no other contact */
@@ -134,6 +140,19 @@
 		}
 
 		details.uri = pjsip_uri_get_uri(contact->uri);
+
+		/* pjsip_uri_print returns -1 if there's not enough room in the buffer */
+		if (pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, details.uri, contact_uri, sizeof(contact_uri)) < 0) {
+			/* If the total length of the uri is greater than pjproject can handle, go no further */
+			pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+			return -1;
+		}
+
+		if (details.uri->host.slen >= pj_max_hostname) {
+			/* If the length of the hostname is greater than pjproject can handle, go no further */
+			pjsip_endpt_release_pool(ast_sip_get_pjsip_endpoint(), details.pool);
+			return -1;
+		}
 
 		/* Determine if this is an add, update, or delete for policy enforcement purposes */
 		if (!(existing = ao2_callback(contacts, 0, registrar_find_contact, &details))) {
@@ -472,7 +491,7 @@
 	/* Iterate each provided Contact header and add, update, or delete */
 	while ((contact_hdr = pjsip_msg_find_hdr(task_data->rdata->msg_info.msg, PJSIP_H_CONTACT, contact_hdr ? contact_hdr->next : NULL))) {
 		int expiration;
-		char contact_uri[PJSIP_MAX_URL_SIZE];
+		char contact_uri[pjsip_max_url_size];
 		RAII_VAR(struct ast_sip_contact *, contact, NULL, ao2_cleanup);
 
 		if (contact_hdr->star) {
@@ -827,6 +846,12 @@
 {
 	const pj_str_t STR_REGISTER = { "REGISTER", 8 };
 
+	CHECK_PJPROJECT_MODULE_LOADED();
+
+	ast_pjproject_get_buildopt("PJ_MAX_HOSTNAME", "%d", &pj_max_hostname);
+	/* As of pjproject 2.4.5, PJSIP_MAX_URL_SIZE isn't exposed yet but we try anyway. */
+	ast_pjproject_get_buildopt("PJSIP_MAX_URL_SIZE", "%d", &pjsip_max_url_size);
+
 	CHECK_PJSIP_MODULE_LOADED();
 
 	if (!(serializers = ao2_container_alloc(

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ea7cee16f29c8088794de3085ca7523c1c4833d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list