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

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


Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/2596

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: Ic4f5e49f1a83fef4951ffeeef8f443a7f6ac15eb
---
A include/asterisk/res_pjproject.h
M include/asterisk/vector.h
A res/res_pjproject.c
A res/res_pjproject.exports.in
M res/res_pjsip/include/res_pjsip_private.h
M res/res_pjsip/location.c
D res/res_pjsip_log_forwarder.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_registrar.c
9 files changed, 514 insertions(+), 127 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/96/2596/1

diff --git a/include/asterisk/res_pjproject.h b/include/asterisk/res_pjproject.h
new file mode 100644
index 0000000..8828b34
--- /dev/null
+++ b/include/asterisk/res_pjproject.h
@@ -0,0 +1,96 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Fairview 5 Engineering, LLC
+ *
+ * George Joseph <george.joseph at fairview5.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+#ifndef _RES_PJPROJECT_H
+#define _RES_PJPROJECT_H
+
+/*! \brief Determines whether the res_pjproject module is loaded */
+#define CHECK_PJPROJECT_MODULE_LOADED()                 \
+	do {                                                \
+		if (!ast_module_check("res_pjproject.so")) {    \
+			return AST_MODULE_LOAD_DECLINE;             \
+		}                                               \
+	} while(0)
+
+/*!
+ * \brief Retrieve a pjproject build option
+ *
+ * \param option The build option requested
+ * \param format_string A scanf-style format string to parse the option value into
+ * \param ... Pointers to variables to receive the values parsed
+ *
+ * \retval The number of values parsed
+ *
+ * \since 13.8.0
+ *
+ * \note The option requested must be from those returned by pj_dump_config()
+ * which can be displayed with the 'pjsip show buildopts' CLI command.
+ *
+ *   <b>Sample Usage:</b>
+ *   \code
+ *
+ *   int max_hostname;
+ *
+ *   ast_sip_get_pjproject_buildopt("PJ_MAX_HOSTNAME", "%d", &max_hostname);
+ *
+ *   \endcode
+ *
+ */
+int ast_pjproject_get_buildopt(char *option, char *format_string, ...) __attribute__((format(scanf, 2, 3)));
+
+/*!
+ * \brief Begin PJPROJECT log interception for CLI output.
+ * \since 13.8.0
+ *
+ * \param fd CLI file descriptior to send intercepted output.
+ *
+ * \note ast_pjproject_log_intercept_begin() and
+ * ast_pjproject_log_intercept_end() must always be called
+ * in pairs.
+ *
+ * \return Nothing
+ */
+void ast_pjproject_log_intercept_begin(int fd);
+
+/*!
+ * \brief End PJPROJECT log interception for CLI output.
+ * \since 13.8.0
+ *
+ * \note ast_pjproject_log_intercept_begin() and
+ * ast_pjproject_log_intercept_end() must always be called
+ * in pairs.
+ *
+ * \return Nothing
+ */
+void ast_pjproject_log_intercept_end(void);
+
+/*!
+ * \brief Increment the res_pjproject reference count.
+ *
+ * This ensures graceful shutdown happens in the proper order.
+ */
+void ast_pjproject_ref(void);
+
+/*!
+ * \brief Decrement the res_pjproject reference count.
+ *
+ * This ensures graceful shutdown happens in the proper order.
+ */
+void ast_pjproject_unref(void);
+
+#endif /* _RES_PJPROJECT_H */
diff --git a/include/asterisk/vector.h b/include/asterisk/vector.h
index 0053d8a..bf61fda 100644
--- a/include/asterisk/vector.h
+++ b/include/asterisk/vector.h
@@ -87,6 +87,31 @@
 } while (0)
 
 /*!
+ * \internal
+ */
+#define __make_room(idx, vec) ({ \
+	int res = 0;								\
+	do {														\
+		if ((idx) >= (vec)->max) {								\
+			size_t new_max = ((idx) + 1) * 2;				\
+			typeof((vec)->elems) new_elems = ast_calloc(1,		\
+				new_max * sizeof(*new_elems));					\
+			if (new_elems) {									\
+				memcpy(new_elems, (vec)->elems,					\
+					(vec)->current * sizeof(*new_elems)); 		\
+				ast_free((vec)->elems);							\
+				(vec)->elems = new_elems;						\
+				(vec)->max = new_max;							\
+			} else {											\
+				res = -1;										\
+				break;											\
+			}													\
+		}														\
+	} while(0);													\
+	res;														\
+})
+
+/*!
  * \brief Append an element to a vector, growing the vector if needed.
  *
  * \param vec Vector to append to.
@@ -158,6 +183,36 @@
 })
 
 /*!
+ * \brief Add an element into a sorted vector
+ *
+ * \param vec Sorted vector to add to.
+ * \param elem Element to insert.
+ * \param cmp A strcmp compatible compare function.
+ *
+ * \return 0 on success.
+ * \return Non-zero on failure.
+ *
+ * \warning Use of this macro on an unsorted vector will produce unpredictable results
+ */
+#define AST_VECTOR_ADD_SORTED(vec, elem, cmp) ({ \
+	int res = 0; \
+	size_t __idx = (vec)->current; \
+	do { \
+		if (__make_room((vec)->current, vec) != 0) { \
+			res = -1; \
+			break; \
+		} \
+		while (__idx > 0 && (cmp((vec)->elems[__idx - 1], elem) > 0)) { \
+			(vec)->elems[__idx] = (vec)->elems[__idx - 1]; \
+			__idx--; \
+		} \
+		(vec)->elems[__idx] = elem; \
+		(vec)->current++; \
+	} while (0); \
+	res; \
+})
+
+/*!
  * \brief Remove an element from a vector by index.
  *
  * Note that elements in the vector may be reordered, so that the remove can
diff --git a/res/res_pjproject.c b/res/res_pjproject.c
new file mode 100644
index 0000000..83940dd
--- /dev/null
+++ b/res/res_pjproject.c
@@ -0,0 +1,256 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2013, Digium, Inc.
+ *
+ * David M. Lee, II <dlee at digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief Bridge PJPROJECT logging to Asterisk logging.
+ * \author David M. Lee, II <dlee at digium.com>
+ *
+ * PJPROJECT logging doesn't exactly match Asterisk logging, but mapping the two is
+ * not too bad. PJPROJECT log levels are identified by a single int. Limits are
+ * not specified by PJPROJECT, but their implementation used 1 through 6.
+ *
+ * The mapping is as follows:
+ *  - 0: LOG_ERROR
+ *  - 1: LOG_ERROR
+ *  - 2: LOG_WARNING
+ *  - 3 and above: equivalent to ast_debug(level, ...) for res_pjproject.so
+ */
+
+/*** MODULEINFO
+	<depend>pjproject</depend>
+	<support_level>core</support_level>
+ ***/
+
+#include "asterisk.h"
+
+#include <stdarg.h>
+#include <pjlib.h>
+#include <pjsip.h>
+#include <pj/log.h>
+
+#include "asterisk/logger.h"
+#include "asterisk/module.h"
+#include "asterisk/cli.h"
+#include "asterisk/res_pjproject.h"
+#include "asterisk/vector.h"
+
+static pj_log_func *log_cb_orig;
+static unsigned decor_orig;
+
+static AST_VECTOR(buildopts, char *) buildopts;
+
+/*! Protection from other log intercept instances.  There can be only one at a time. */
+AST_MUTEX_DEFINE_STATIC(pjproject_log_intercept_lock);
+
+struct pjproject_log_intercept_data {
+	pthread_t thread;
+	int fd;
+};
+
+static struct pjproject_log_intercept_data pjproject_log_intercept = {
+	.thread = AST_PTHREADT_NULL,
+	.fd = -1,
+};
+
+static void log_forwarder(int level, const char *data, int len)
+{
+	int ast_level;
+	/* PJPROJECT doesn't provide much in the way of source info */
+	const char * log_source = "pjproject";
+	int log_line = 0;
+	const char *log_func = "<?>";
+	int mod_level;
+
+	if (pjproject_log_intercept.fd != -1
+		&& pjproject_log_intercept.thread == pthread_self()) {
+		/*
+		 * We are handling a CLI command intercepting PJPROJECT
+		 * log output.
+		 */
+		ast_cli(pjproject_log_intercept.fd, "%s\n", data);
+		return;
+	}
+
+	/* Lower number indicates higher importance */
+	switch (level) {
+	case 0: /* level zero indicates fatal error, according to docs */
+	case 1: /* 1 seems to be used for errors */
+		ast_level = __LOG_ERROR;
+		break;
+	case 2: /* 2 seems to be used for warnings and errors */
+		ast_level = __LOG_WARNING;
+		break;
+	default:
+		ast_level = __LOG_DEBUG;
+
+		/* For levels 3 and up, obey the debug level for res_pjproject */
+		mod_level = ast_opt_dbg_module ?
+			ast_debug_get_by_module("res_pjproject") : 0;
+		if (option_debug < level && mod_level < level) {
+			return;
+		}
+		break;
+	}
+
+	/* PJPROJECT uses indention to indicate function call depth. We'll prepend
+	 * log statements with a tab so they'll have a better shot at lining
+	 * up */
+	ast_log(ast_level, log_source, log_line, log_func, "\t%s\n", data);
+}
+
+static void capture_buildopts_cb(int level, const char *data, int len)
+{
+	if (strstr(data, "Teluu") || strstr(data, "Dumping")) {
+		return;
+	}
+
+	AST_VECTOR_ADD_SORTED(&buildopts, ast_strdup(ast_skip_blanks(data)), strcmp);
+}
+
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
+int ast_pjproject_get_buildopt(char *option, char *format_string, ...)
+{
+	int res = 0;
+	char *format_temp;
+	int i;
+
+	format_temp = ast_alloca(strlen(option) + strlen(" : ") + strlen(format_string) + 1);
+	sprintf(format_temp, "%s : %s", option, format_string);
+
+	for (i = 0; i < AST_VECTOR_SIZE(&buildopts); i++) {
+		va_list arg_ptr;
+		va_start(arg_ptr, format_string);
+		res = vsscanf(AST_VECTOR_GET(&buildopts, i), format_temp, arg_ptr);
+		va_end(arg_ptr);
+		if (res) {
+			break;
+		}
+	}
+
+	return res;
+}
+#pragma GCC diagnostic warning "-Wformat-nonliteral"
+
+void ast_pjproject_log_intercept_begin(int fd)
+{
+	/* Protect from other CLI instances trying to do this at the same time. */
+	ast_mutex_lock(&pjproject_log_intercept_lock);
+
+	pjproject_log_intercept.thread = pthread_self();
+	pjproject_log_intercept.fd = fd;
+}
+
+void ast_pjproject_log_intercept_end(void)
+{
+	pjproject_log_intercept.fd = -1;
+	pjproject_log_intercept.thread = AST_PTHREADT_NULL;
+
+	ast_mutex_unlock(&pjproject_log_intercept_lock);
+}
+
+void ast_pjproject_ref(void)
+{
+	ast_module_ref(ast_module_info->self);
+}
+
+void ast_pjproject_unref(void)
+{
+	ast_module_unref(ast_module_info->self);
+}
+
+static char *handle_pjproject_show_buildopts(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
+{
+	int i;
+
+	switch (cmd) {
+	case CLI_INIT:
+		e->command = "pjproject show buildopts";
+		e->usage =
+			"Usage: pjproject show buildopts\n"
+			"       Show the compile time config of the pjproject that Asterisk is\n"
+			"       running against.\n";
+		return NULL;
+	case CLI_GENERATE:
+		return NULL;
+	}
+
+	ast_cli(a->fd, "PJPROJECT compile time config currently running against:\n");
+
+	for (i = 0; i < AST_VECTOR_SIZE(&buildopts); i++) {
+		ast_cli(a->fd, "%s\n", AST_VECTOR_GET(&buildopts, i));
+	}
+
+	return CLI_SUCCESS;
+}
+
+static struct ast_cli_entry pjproject_cli[] = {
+	AST_CLI_DEFINE(handle_pjproject_show_buildopts, "Show the compiled config of the pjproject in use"),
+};
+
+static int load_module(void)
+{
+	ast_debug(3, "Starting PJPROJECT logging to Asterisk logger\n");
+
+	pj_init();
+
+	decor_orig = pj_log_get_decor();
+	log_cb_orig = pj_log_get_log_func();
+
+	if (AST_VECTOR_INIT(&buildopts, 64)) {
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	/*
+	 * On startup, we want to capture the dump once and store it.
+	 */
+	pj_log_set_log_func(capture_buildopts_cb);
+	pj_log_set_decor(0);
+	pj_dump_config();
+	pj_log_set_decor(PJ_LOG_HAS_SENDER | PJ_LOG_HAS_INDENT);
+	pj_log_set_log_func(log_forwarder);
+
+	ast_cli_register_multiple(pjproject_cli, ARRAY_LEN(pjproject_cli));
+
+	return AST_MODULE_LOAD_SUCCESS;
+}
+
+#define NOT_EQUALS(a, b) (a != b)
+
+static int unload_module(void)
+{
+	ast_cli_unregister_multiple(pjproject_cli, ARRAY_LEN(pjproject_cli));
+	pj_log_set_log_func(log_cb_orig);
+	pj_log_set_decor(decor_orig);
+
+	AST_VECTOR_REMOVE_CMP_UNORDERED(&buildopts, NULL, NOT_EQUALS, ast_free);
+	AST_VECTOR_FREE(&buildopts);
+
+	ast_debug(3, "Stopped PJPROJECT logging to Asterisk logger\n");
+
+	pj_shutdown();
+
+	return 0;
+}
+
+AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS | AST_MODFLAG_LOAD_ORDER, "PJPROJECT Log and Utility Support",
+	.support_level = AST_MODULE_SUPPORT_CORE,
+	.load = load_module,
+	.unload = unload_module,
+	.load_pri = AST_MODPRI_CHANNEL_DEPEND - 6,
+	);
diff --git a/res/res_pjproject.exports.in b/res/res_pjproject.exports.in
new file mode 100644
index 0000000..81d55db
--- /dev/null
+++ b/res/res_pjproject.exports.in
@@ -0,0 +1,7 @@
+{
+    global:
+        LINKER_SYMBOL_PREFIXast_pjproject_*;
+    local:
+        *;
+};
+
diff --git a/res/res_pjsip/include/res_pjsip_private.h b/res/res_pjsip/include/res_pjsip_private.h
index fa37c8c..100f466 100644
--- a/res/res_pjsip/include/res_pjsip_private.h
+++ b/res/res_pjsip/include/res_pjsip_private.h
@@ -112,4 +112,10 @@
 int ast_sip_initialize_cli(void);
 void ast_sip_destroy_cli(void);
 
+/*!
+ * \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 29f186c..837ade0 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -26,6 +26,10 @@
 #include "asterisk/sorcery.h"
 #include "include/res_pjsip_private.h"
 #include "asterisk/res_pjsip_cli.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)
@@ -266,6 +270,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)
 {
@@ -282,6 +323,11 @@
 	while ((contact_uri = strsep(&contacts, ","))) {
 		struct ast_sip_contact *contact;
 		char contact_id[strlen(aor_id) + strlen(contact_uri) + 2 + 1];
+
+		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) {
 			aor->permanent_contacts = ao2_container_alloc_list(AO2_ALLOC_OPT_LOCK_NOLOCK,
@@ -811,6 +857,11 @@
 int ast_sip_initialize_sorcery_location(void)
 {
 	struct ast_sorcery *sorcery = ast_sip_get_sorcery();
+
+	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_log_forwarder.c b/res/res_pjsip_log_forwarder.c
deleted file mode 100644
index 7b095bb..0000000
--- a/res/res_pjsip_log_forwarder.c
+++ /dev/null
@@ -1,125 +0,0 @@
-/*
- * Asterisk -- An open source telephony toolkit.
- *
- * Copyright (C) 2013, Digium, Inc.
- *
- * David M. Lee, II <dlee at digium.com>
- *
- * See http://www.asterisk.org for more information about
- * the Asterisk project. Please do not directly contact
- * any of the maintainers of this project for assistance;
- * the project provides a web site, mailing lists and IRC
- * channels for your use.
- *
- * This program is free software, distributed under the terms of
- * the GNU General Public License Version 2. See the LICENSE file
- * at the top of the source tree.
- */
-
-/*! \file
- *
- * \brief Bridge PJSIP logging to Asterisk logging.
- * \author David M. Lee, II <dlee at digium.com>
- *
- * PJSIP logging doesn't exactly match Asterisk logging, but mapping the two is
- * not too bad. PJSIP log levels are identified by a single int. Limits are
- * not specified by PJSIP, but their implementation used 1 through 6.
- *
- * The mapping is as follows:
- *  - 0: LOG_ERROR
- *  - 1: LOG_ERROR
- *  - 2: LOG_WARNING
- *  - 3 and above: equivalent to ast_debug(level, ...) for res_pjsip.so
- */
-
-/*** MODULEINFO
-	<depend>pjproject</depend>
-	<support_level>core</support_level>
- ***/
-
-#include "asterisk.h"
-
-ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
-
-#include <pjsip.h>
-#include <pj/log.h>
-
-#include "asterisk/logger.h"
-#include "asterisk/module.h"
-
-static pj_log_func *log_cb_orig;
-static unsigned decor_orig;
-
-static void log_cb(int level, const char *data, int len)
-{
-	int ast_level;
-	/* PJSIP doesn't provide much in the way of source info */
-	const char * log_source = "pjsip";
-	int log_line = 0;
-	const char *log_func = "<?>";
-	int mod_level;
-
-	/* Lower number indicates higher importance */
-	switch (level) {
-	case 0: /* level zero indicates fatal error, according to docs */
-	case 1: /* 1 seems to be used for errors */
-		ast_level = __LOG_ERROR;
-		break;
-	case 2: /* 2 seems to be used for warnings and errors */
-		ast_level = __LOG_WARNING;
-		break;
-	default:
-		ast_level = __LOG_DEBUG;
-
-		/* For levels 3 and up, obey the debug level for res_pjsip */
-		mod_level = ast_opt_dbg_module ?
-			ast_debug_get_by_module("res_pjsip") : 0;
-		if (option_debug < level && mod_level < level) {
-			return;
-		}
-		break;
-	}
-
-	/* PJSIP uses indention to indicate function call depth. We'll prepend
-	 * log statements with a tab so they'll have a better shot at lining
-	 * up */
-	ast_log(ast_level, log_source, log_line, log_func, "\t%s\n", data);
-}
-
-static int load_module(void)
-{
-	pj_init();
-
-	decor_orig = pj_log_get_decor();
-	log_cb_orig = pj_log_get_log_func();
-
-	ast_debug(3, "Forwarding PJSIP logger to Asterisk logger\n");
-	/* SENDER prepends the source to the log message. This could be a
-	 * filename, object reference, or simply a string
-	 *
-	 * INDENT is assumed to be on by most log statements in PJSIP itself.
-	 */
-	pj_log_set_decor(PJ_LOG_HAS_SENDER | PJ_LOG_HAS_INDENT);
-	pj_log_set_log_func(log_cb);
-
-	return AST_MODULE_LOAD_SUCCESS;
-}
-
-static int unload_module(void)
-{
-	pj_log_set_log_func(log_cb_orig);
-	pj_log_set_decor(decor_orig);
-
-	pj_shutdown();
-
-	return 0;
-}
-
-/* While we don't really export global symbols, we want to load before other
- * modules that do */
-AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS | AST_MODFLAG_LOAD_ORDER, "PJSIP Log Forwarder",
-	.support_level = AST_MODULE_SUPPORT_CORE,
-	.load = load_module,
-	.unload = unload_module,
-	.load_pri = AST_MODPRI_CHANNEL_DEPEND - 6,
-	);
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 8585b19..3436db4 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -1070,10 +1070,26 @@
 		ast_log(LOG_ERROR, "No server URI specified on outbound registration '%s'",
 			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));
+		return -1;
+	} else if (!ast_strlen_zero(applied->endpoint) && !applied->line) {
+		ast_log(LOG_ERROR, "An endpoint has been specified on outbound registration '%s' without enabling line support\n",
+			ast_sorcery_object_get_id(applied));
+		return -1;
 	}
 
 	if (state && can_reuse_registration(state->registration, applied)) {
diff --git a/res/res_pjsip_registrar.c b/res/res_pjsip_registrar.c
index 595f834..a4a75bb 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>
  ***/
@@ -32,6 +33,7 @@
 #include "asterisk/test.h"
 #include "asterisk/taskprocessor.h"
 #include "asterisk/manager.h"
+#include "asterisk/res_pjproject.h"
 #include "res_pjsip/include/res_pjsip_private.h"
 
 /*** DOCUMENTATION
@@ -50,6 +52,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)
@@ -85,7 +90,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 */
@@ -113,6 +118,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 */
@@ -133,6 +139,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))) {
@@ -471,7 +490,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) {
@@ -788,6 +807,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/2596
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic4f5e49f1a83fef4951ffeeef8f443a7f6ac15eb
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list