From: Liviu Chircu <liviu@opensips.org>
Date: Thu, 24 May 2018 17:25:41 +0300
Subject: [PATCH] path: Add escaping logic for add_path_received()

Simply dumping a SIP URI as a ";received=" Path header field parameter
value is problematic and leads to two kinds of issues:

a. invalid SIP URIs, where the ";transport=" is specified twice:

Path: <sip:127.0.0.1:5061;transport=tcp;lr; ...
       ... received=sip:127.0.0.1:51959;transport=tcp>

b. in double Path scenarios (e.g. when switching protocols from, say,
TCP -> UDP), since the ";received=" is inserted as the
inbound Path header field value, this can lead to a strange
"protocol conversion" effect:

Path: <sip:127.0.0.1:5061;r2=on;lr ...
       ... ;received=sip:127.0.0.1:42917;transport=tcp>
Path: <sip:127.0.0.1:5061;transport=tcp;r2=on;lr>

^ notice how the inbound Path intended to advertise an UDP URI, but the
add_path_received() managed to "convert" it into a TCP URI because of
its inability to escape its token. nathelper is an easy first victim of
this conversion, as it will attempt to send pings using the incorrect
transport after reading the topmost Path (Route) header.

This patch adds escaping/unescaping logic for the ";received=" feature
of add_path_received() along with any code that makes use of this
feature. Example resulting URI for the above example:

Path: <sip:127.0.0.1:5061;r2=on;lr ...
       ... ;received=sip:127.0.0.1:42917%3dtransport%3btcp>

(cherry picked from commit b3bf15646affe981d4b266381d59f210e6e882fd)
(cherry picked from commit 6e0ec1f7bae4b5a5076fce445d28ba1cbf790feb)

diff --git a/modules/path/path.c b/modules/path/path.c
index 42cb5ed7f..f96b10150 100644
--- a/modules/path/path.c
+++ b/modules/path/path.c
@@ -28,6 +28,8 @@
 #include "../../mem/mem.h"
 #include "../../data_lump.h"
 #include "../../parser/parse_param.h"
+#include "../../strcommon.h"
+#include "../../ut.h"
 
 #include "path.h"
 #include "path_mod.h"
@@ -44,6 +46,9 @@
 #define PATH_TRANS_PARAM	";transport="
 #define PATH_TRANS_PARAM_LEN	(sizeof(PATH_TRANS_PARAM)-1)
 
+#define PATH_ESC_TRANS_PARAM	"\%3btransport\%3d"
+#define PATH_ESC_TRANS_PARAM_LEN	(sizeof(PATH_ESC_TRANS_PARAM)-1)
+
 #define	PATH_CRLF		">\r\n"
 #define PATH_CRLF_LEN		(sizeof(PATH_CRLF)-1)
 
@@ -127,16 +132,16 @@ static int build_path(struct sip_msg* _m, struct lump* l, struct lump* l2,
 		rcv_addr.len = snprintf(rcv_addr.s, 4 + IP_ADDR_MAX_STR_SIZE + 6, "sip:%s:%u", src_ip, _m->rcv.src_port);
 		switch (_m->rcv.proto) {
 			case PROTO_TCP:
-				memcpy(rcv_addr.s+rcv_addr.len, PATH_TRANS_PARAM "tcp",PATH_TRANS_PARAM_LEN+3);
-				rcv_addr.len += PATH_TRANS_PARAM_LEN + 3;
+				memcpy(rcv_addr.s+rcv_addr.len, PATH_ESC_TRANS_PARAM "tcp",PATH_ESC_TRANS_PARAM_LEN+3);
+				rcv_addr.len += PATH_ESC_TRANS_PARAM_LEN + 3;
 				break;
 			case PROTO_TLS:
-				memcpy(rcv_addr.s+rcv_addr.len, PATH_TRANS_PARAM "tls",PATH_TRANS_PARAM_LEN+3);
-				rcv_addr.len += PATH_TRANS_PARAM_LEN + 3;
+				memcpy(rcv_addr.s+rcv_addr.len, PATH_ESC_TRANS_PARAM "tls",PATH_ESC_TRANS_PARAM_LEN+3);
+				rcv_addr.len += PATH_ESC_TRANS_PARAM_LEN + 3;
 				break;
 			case PROTO_SCTP:
-				memcpy(rcv_addr.s+rcv_addr.len, PATH_TRANS_PARAM "sctp",PATH_TRANS_PARAM_LEN+4);
-				rcv_addr.len += PATH_TRANS_PARAM_LEN + 4;
+				memcpy(rcv_addr.s+rcv_addr.len, PATH_ESC_TRANS_PARAM "sctp",PATH_ESC_TRANS_PARAM_LEN+4);
+				rcv_addr.len += PATH_ESC_TRANS_PARAM_LEN + 4;
 				break;
 		}
 		l2 = insert_new_lump_before(l2, rcv_addr.s, rcv_addr.len, 0);
@@ -273,12 +278,16 @@ int add_path_received_usr(struct sip_msg* _msg, char* _usr, char* _b)
  */
 void path_rr_callback(struct sip_msg *_m, str *r_param, void *cb_param)
 {
+	static char _unescape_buf[MAX_PATH_SIZE];
+
 	param_hooks_t hooks;
 	param_t *params;
 	param_t *first_param;
 	str received = {0, 0};
 	str transport = {0, 0};
 	str dst_uri = {0, 0};
+	str unescape_buf = {_unescape_buf, MAX_PATH_SIZE};
+	char *p;
 
 	if (parse_params(r_param, CLASS_ANY, &hooks, &params) != 0) {
 		LM_ERR("failed to parse route parameters\n");
@@ -289,15 +298,36 @@ void path_rr_callback(struct sip_msg *_m, str *r_param, void *cb_param)
 
 	while(params)
 	{
-		if ( params->name.len == 9 && !strncasecmp(params->name.s, "transport", params->name.len) )
-			transport = params->body;
+		if (params->name.len == 8 &&
+		    !strncasecmp(params->name.s, "received", params->name.len)) {
 
-		if ( params->name.len == 8 && !strncasecmp(params->name.s,"received", params->name.len) )
 			received = params->body;
+			unescape_buf.len = MAX_PATH_SIZE;
+			if (unescape_param(&received, &unescape_buf) != 0) {
+				LM_ERR("failed to unescape received=%.*s\n",
+				       received.len, received.s);
+				goto out1;
+			}
+
+			/* if there's a param here, it has to be ;transport= */
+			if ((p = q_memchr(unescape_buf.s, ';', unescape_buf.len))) {
+				received.len = p - unescape_buf.s;
+
+				if ((p = q_memchr(p, '=', unescape_buf.len))) {
+					transport.s = p + 1;
+					transport.len = unescape_buf.s + unescape_buf.len - transport.s;
+				}
+			}
+
+			break;
+		}
 
 		params = params->next;
 	}
 
+	LM_DBG("extracted received=%.*s, transport=%.*s\n",
+	       received.len, received.s, transport.len, transport.s);
+
 	if (received.len > 0) {
 		if (transport.len > 0) {
 			dst_uri.len = received.len + PATH_TRANS_PARAM_LEN + 1 + transport.len;
diff --git a/modules/registrar/path.c b/modules/registrar/path.c
index a48c69dc3..02ff6833d 100644
--- a/modules/registrar/path.c
+++ b/modules/registrar/path.c
@@ -30,6 +30,9 @@
 #include "../../data_lump.h"
 #include "../../parser/parse_rr.h"
 #include "../../parser/parse_uri.h"
+#include "../../ut.h"
+#include "../../strcommon.h"
+
 #include "path.h"
 #include "reg_mod.h"
 
@@ -40,9 +43,12 @@ int build_path_vector(struct sip_msg *_m, str *path, str *received,
 														unsigned int flags)
 {
 	static char buf[MAX_PATH_SIZE];
+	static char _unescape_buf[MAX_PATH_SIZE];
+
 	char *p;
 	struct hdr_field *hdr;
 	struct sip_uri puri;
+	str unescape_buf = {_unescape_buf, MAX_PATH_SIZE};
 
 	rr_t *route = 0;
 
@@ -91,14 +97,18 @@ int build_path_vector(struct sip_msg *_m, str *path, str *received,
 				LM_ERR("failed to parse parameters of first hop\n");
 				goto error;
 			}
-			if (hooks.contact.received)
-				*received = hooks.contact.received->body;
-			/*for (;params; params = params->next) {
-				if (params->type == P_RECEIVED) {
-					*received = hooks.contact.received->body;
-					break;
-				}
-			}*/
+
+			if (hooks.contact.received) {
+				unescape_buf.len = MAX_PATH_SIZE;
+				if (unescape_param(&hooks.contact.received->body,
+				                           &unescape_buf) != 0)
+					LM_ERR("failed to unescape received=%.*s\n",
+					       hooks.contact.received->body.len,
+					       hooks.contact.received->body.s);
+				else
+					*received = unescape_buf;
+			}
+
 			free_params(params);
 		}
 		free_rr(&route);