[Asterisk-code-review] res pjsip refer.c: Fix seg fault in process of Refer-to header. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_pjsip_refer.c: Fix seg fault in process of Refer-to header.
......................................................................


res_pjsip_refer.c: Fix seg fault in process of Refer-to header.

The "Refer-to" header of an incoming REFER request is parsed by
pjsip_parse_uri().  That function requires the URI parameter to be NULL
terminated.  Unfortunately, the previous code added the NULL terminator by
overwriting memory that may not be safe.  The overwritten memory results
could be benign, memory corruption, or a segmentation fault.  Now the URI
is NULL terminated safely by copying the URI to a new chunk of memory with
the correct size to be NULL terminated.

ASTERISK-25814 #close

Change-Id: I32565496684a5a49c3278fce06474b8c94b37342
---
M res/res_pjsip_refer.c
1 file changed, 9 insertions(+), 9 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/res/res_pjsip_refer.c b/res/res_pjsip_refer.c
index 3766f9b..f3af65c 100644
--- a/res/res_pjsip_refer.c
+++ b/res/res_pjsip_refer.c
@@ -985,6 +985,7 @@
 {
 	pjsip_generic_string_hdr *refer_to;
 	char *uri;
+	size_t uri_size;
 	pjsip_uri *target;
 	pjsip_sip_uri *target_uri;
 	RAII_VAR(struct refer_progress *, progress, NULL, ao2_cleanup);
@@ -1018,20 +1019,19 @@
 		return 0;
 	}
 
-	/* This is done on purpose (and is safe) - it's done so that the value passed to
-	 * pjsip_parse_uri is NULL terminated as required
+	/* The ast_copy_pj_str to uri is needed because it puts the NULL terminator to the uri
+	 * as pjsip_parse_uri require a NULL terminated uri
 	 */
-	uri = refer_to->hvalue.ptr;
-	uri[refer_to->hvalue.slen] = '\0';
 
-	target = pjsip_parse_uri(rdata->tp_info.pool, refer_to->hvalue.ptr, refer_to->hvalue.slen, 0);
+	uri_size = pj_strlen(&refer_to->hvalue) + 1;
+	uri = ast_alloca(uri_size);
+	ast_copy_pj_str(uri, &refer_to->hvalue, uri_size);
+
+	target = pjsip_parse_uri(rdata->tp_info.pool, uri, uri_size - 1, 0);
+
 	if (!target
 		|| (!PJSIP_URI_SCHEME_IS_SIP(target)
 			&& !PJSIP_URI_SCHEME_IS_SIPS(target))) {
-		size_t uri_size = pj_strlen(&refer_to->hvalue) + 1;
-		char *uri = ast_alloca(uri_size);
-
-		ast_copy_pj_str(uri, &refer_to->hvalue, uri_size);
 
 		pjsip_dlg_respond(session->inv_session->dlg, rdata, 400, NULL, NULL, NULL);
 		ast_debug(3, "Received a REFER without a parseable Refer-To ('%s') on channel '%s' from endpoint '%s'\n",

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I32565496684a5a49c3278fce06474b8c94b37342
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Sergio Medina Toledo <lumasepa at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>