[Asterisk-code-review] res_pjsip_nat.c: Create deep copies of strings when appropriate (asterisk[16])

George Joseph asteriskteam at digium.com
Thu Dec 17 09:10:43 CST 2020


George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15209 )

Change subject: res_pjsip_nat.c: Create deep copies of strings when appropriate
......................................................................

res_pjsip_nat.c: Create deep copies of strings when appropriate

In rewrite_uri asterisk was not making deep copies of strings when
changing the uri. This was in some cases causing garbage in the route
header and in other cases even crashing asterisk when receiving a
message with a record-route header set. Thanks to Ralf Kubis for
pointing out why this happens. A similar problem was found in
res_pjsip_transport_websocket.c. Pjproject needs as well to be patched
to avoid garbage in CANCEL messages.

ASTERISK-29024 #close

Change-Id: Ic5acd7fa2fbda3080f5f36ef12e46804939b198b
---
M res/res_pjsip_nat.c
M res/res_pjsip_transport_websocket.c
A third-party/pjproject/patches/0070-fix-incorrect-copying-when-creating-cancel.patch
3 files changed, 43 insertions(+), 6 deletions(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index 9dab32a..3d6f25d 100644
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -66,14 +66,14 @@
 	return;
 }
 
-static void rewrite_uri(pjsip_rx_data *rdata, pjsip_sip_uri *uri)
+static void rewrite_uri(pjsip_rx_data *rdata, pjsip_sip_uri *uri, pj_pool_t *pool)
 {
 
 	if (pj_strcmp2(&uri->host, rdata->pkt_info.src_name) != 0) {
 		save_orig_contact_host(rdata, uri);
 	}
 
-	pj_cstr(&uri->host, rdata->pkt_info.src_name);
+	pj_strdup2(pool, &uri->host, rdata->pkt_info.src_name);
 	uri->port = rdata->pkt_info.src_port;
 	if (!strcasecmp("WSS", rdata->tp_info.transport->type_name)) {
 		/* WSS is special, we don't want to overwrite the URI at all as it needs to be ws */
@@ -151,14 +151,14 @@
 
 	if (rr) {
 		uri = pjsip_uri_get_uri(&rr->name_addr);
-		rewrite_uri(rdata, uri);
+		rewrite_uri(rdata, uri, rdata->tp_info.pool);
 		res = 0;
 	}
 
 	if (dlg && !pj_list_empty(&dlg->route_set) && !dlg->route_set_frozen) {
 		pjsip_routing_hdr *route = dlg->route_set.next;
 		uri = pjsip_uri_get_uri(&route->name_addr);
-		rewrite_uri(rdata, uri);
+		rewrite_uri(rdata, uri, dlg->pool);
 		res = 0;
 	}
 
@@ -184,7 +184,7 @@
 	if (contact && !contact->star && (PJSIP_URI_SCHEME_IS_SIP(contact->uri) || PJSIP_URI_SCHEME_IS_SIPS(contact->uri))) {
 		pjsip_sip_uri *uri = pjsip_uri_get_uri(contact->uri);
 
-		rewrite_uri(rdata, uri);
+		rewrite_uri(rdata, uri, rdata->tp_info.pool);
 
 		if (dlg && pj_list_empty(&dlg->route_set) && (!dlg->remote.contact
 			|| pjsip_uri_cmp(PJSIP_URI_IN_REQ_URI, dlg->remote.contact->uri, contact->uri))) {
diff --git a/res/res_pjsip_transport_websocket.c b/res/res_pjsip_transport_websocket.c
index 4f47a8c..1b882da 100644
--- a/res/res_pjsip_transport_websocket.c
+++ b/res/res_pjsip_transport_websocket.c
@@ -454,7 +454,7 @@
 				pj_strbuf(txp_str));
 		}
 
-		pj_cstr(&uri->host, rdata->pkt_info.src_name);
+		pj_strdup2(rdata->tp_info.pool, &uri->host, rdata->pkt_info.src_name);
 		uri->port = rdata->pkt_info.src_port;
 		pj_strdup(rdata->tp_info.pool, &uri->transport_param, txp_str);
 	}
diff --git a/third-party/pjproject/patches/0070-fix-incorrect-copying-when-creating-cancel.patch b/third-party/pjproject/patches/0070-fix-incorrect-copying-when-creating-cancel.patch
new file mode 100644
index 0000000..95725c1
--- /dev/null
+++ b/third-party/pjproject/patches/0070-fix-incorrect-copying-when-creating-cancel.patch
@@ -0,0 +1,37 @@
+From ce18018cc17bef8f80c08686e3a7b28384ef3ba5 Mon Sep 17 00:00:00 2001
+From: sauwming <ming at teluu.com>
+Date: Mon, 12 Oct 2020 13:31:25 +0800
+Subject: [PATCH] Fix incorrect copying of destination info when creating
+ CANCEL (#2546)
+
+---
+ pjsip/src/pjsip/sip_util.c | 10 +++++-----
+ 1 file changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/pjsip/src/pjsip/sip_util.c b/pjsip/src/pjsip/sip_util.c
+index d10a6fa30..a1bf878ea 100644
+--- a/pjsip/src/pjsip/sip_util.c
++++ b/pjsip/src/pjsip/sip_util.c
+@@ -779,14 +779,14 @@ PJ_DEF(pj_status_t) pjsip_endpt_create_cancel( pjsip_endpoint *endpt,
+ 	    pjsip_hdr_clone(cancel_tdata->pool, req_tdata->saved_strict_route);
+     }
+ 
+-    /* Copy the destination host name from the original request */
+-    pj_strdup(cancel_tdata->pool, &cancel_tdata->dest_info.name,
+-	      &req_tdata->dest_info.name);
+-
+-    /* Finally copy the destination info from the original request */
++    /* Copy the destination info from the original request */
+     pj_memcpy(&cancel_tdata->dest_info, &req_tdata->dest_info,
+ 	      sizeof(req_tdata->dest_info));
+ 
++    /* Finally, copy the destination host name from the original request */
++    pj_strdup(cancel_tdata->pool, &cancel_tdata->dest_info.name,
++	      &req_tdata->dest_info.name);
++
+     /* Done.
+      * Return the transmit buffer containing the CANCEL request.
+      */
+-- 
+2.25.1
+

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15209
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ic5acd7fa2fbda3080f5f36ef12e46804939b198b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 5
Gerrit-Owner: nappsoft <infos at nappsoft.ch>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-CC: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20201217/854777a2/attachment-0001.html>


More information about the asterisk-code-review mailing list