[Asterisk-code-review] res pjsip: Fix crash with from user containing invalid chara... (asterisk[13])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5974 )

Change subject: res_pjsip: Fix crash with from_user containing invalid characters.
......................................................................

res_pjsip: Fix crash with from_user containing invalid characters.

If the from_user field contains certain characters (like @, {, ^, etc.),
PJSIP will return a null value for the URI when attempting to parse it.
This causes a crash when trying to dial out through a trunk that contains
these invalid characters in its from_user field.

This change checks the configuration and ensures that an endpoint will
not be created if the from_user contains an invalid character. It also
adds a null check to the PJSIP URI parsing as a backup.

ASTERISK-27036 #close
Reported by: Maxim Vasilev

Change-Id: I0396fdb5080604e0bdf1277464d5c8a85db913d0
---
M res/res_pjsip.c
M res/res_pjsip/pjsip_configuration.c
2 files changed, 40 insertions(+), 1 deletion(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 02d24e9..258713d 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3075,6 +3075,14 @@
 	/* Update the dialog with the new local URI, we do it afterwards so we can use the dialog pool for construction */
 	pj_strdup_with_null(dlg->pool, &dlg->local.info_str, &local_uri);
 	dlg->local.info->uri = pjsip_parse_uri(dlg->pool, dlg->local.info_str.ptr, dlg->local.info_str.slen, 0);
+	if (!dlg->local.info->uri) {
+		ast_log(LOG_ERROR,
+			"Could not parse URI '%s' for endpoint '%s'\n",
+			dlg->local.info_str.ptr, ast_sorcery_object_get_id(endpoint));
+		dlg->sess_count--;
+		pjsip_dlg_terminate(dlg);
+		return NULL;
+	}
 
 	dlg->local.contact = pjsip_parse_hdr(dlg->pool, &HCONTACT, local_uri.ptr, local_uri.slen, NULL);
 
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 9604ff2..893c81e 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -1150,6 +1150,37 @@
 	return 0;
 }
 
+static int from_user_handler(const struct aco_option *opt,
+	struct ast_variable *var, void *obj)
+{
+	struct ast_sip_endpoint *endpoint = obj;
+	/* Valid non-alphanumeric characters for URI */
+	char *valid_uri_marks = "-_.!~*`()";
+	const char *val;
+
+	for (val = var->value; *val; val++) {
+		if (!strchr(valid_uri_marks, *val) && !isdigit(*val) && !isalpha(*val)) {
+			ast_log(LOG_ERROR, "Error configuring endpoint '%s' - '%s' field "
+			"contains invalid character '%c'\n",
+			ast_sorcery_object_get_id(endpoint), var->name, *val);
+			return -1;
+		}
+	}
+
+	ast_string_field_set(endpoint, fromuser, var->value);
+
+	return 0;
+}
+
+static int from_user_to_str(const void *obj, const intptr_t *args, char **buf)
+{
+	const struct ast_sip_endpoint *endpoint = obj;
+
+	*buf = ast_strdup(endpoint->fromuser);
+
+	return 0;
+}
+
 static int set_var_handler(const struct aco_option *opt,
 	struct ast_variable *var, void *obj)
 {
@@ -1914,7 +1945,7 @@
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "cos_video", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, media.cos_video));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "allow_subscribe", "yes", OPT_BOOL_T, 1, FLDSET(struct ast_sip_endpoint, subscription.allow));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "sub_min_expiry", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_endpoint, subscription.minexpiry));
-	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "from_user", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, fromuser));
+	ast_sorcery_object_field_register_custom(sip_sorcery, "endpoint", "from_user", "", from_user_handler, from_user_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "from_domain", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, fromdomain));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "mwi_from_user", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, subscription.mwi.fromuser));
 	ast_sorcery_object_field_register(sip_sorcery, "endpoint", "rtp_engine", "asterisk", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, media.rtp.engine));

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: I0396fdb5080604e0bdf1277464d5c8a85db913d0
Gerrit-Change-Number: 5974
Gerrit-PatchSet: 4
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170711/15d04d9b/attachment.html>