[Asterisk-code-review] res_pjsip_diversion: Fix adding more than one histinfo to Supported (asterisk[13])

[George Joseph]

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

Change subject: res_pjsip_diversion: Fix adding more than one histinfo to Supported
......................................................................

res_pjsip_diversion: Fix adding more than one histinfo to Supported

New responses sent within a PJSIP sessions are based on those that were
sent before. Therefore, adding/modifying a header once causes it to be
sent on all responses that follow.

Sending 181 Call Is Being Forwarded many times first adds "histinfo"
duplicated more and more, and eventually overflows past the array
boundary.

This commit adds a check preventing adding "histinfo" more than once,
and skipping it if there is no more space in the header.

Similar overflow situations can also occur in res_pjsip_path and
res_pjsip_outbound_registration so those were also modified to
check the bounds and suppress duplicate Supported values.

ASTERISK-29227
Reported by: Ivan Poddubny

Change-Id: Id43704a1f1a0293e35cc7f844026f0b04f2ac322
---
M res/res_pjsip_diversion.c
M res/res_pjsip_outbound_registration.c
M res/res_pjsip_path.c
3 files changed, 38 insertions(+), 0 deletions(-)

Approvals:
  Ivan Poddubny: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved; Approved for Submit
  Friendly Automation: Verified



diff --git a/res/res_pjsip_diversion.c b/res/res_pjsip_diversion.c
index 08f75e0..d344214 100644
--- a/res/res_pjsip_diversion.c
+++ b/res/res_pjsip_diversion.c
@@ -120,6 +120,7 @@
 static int add_supported(pjsip_tx_data *tdata)
 {
 	pjsip_supported_hdr *hdr;
+	unsigned int i;
 
 	hdr = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_SUPPORTED, NULL);
 	if (!hdr) {
@@ -132,6 +133,19 @@
 		pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *)hdr);
 	}
 
+	/* Asterisk can send multiple "181 Call forwarded" in a single session,
+	 * we might have already modified Supported before
+	 */
+	for (i = 0; i < hdr->count; ++i) {
+		if (pj_stricmp(&hdr->values[i], &HISTINFO_SUPPORTED_NAME) == 0) {
+			return 0;
+		}
+	}
+
+	if (hdr->count >= PJSIP_GENERIC_ARRAY_MAX_COUNT) {
+		return -1;
+	}
+
 	/* add on to the existing Supported header */
 	pj_strassign(&hdr->values[hdr->count++], &HISTINFO_SUPPORTED_NAME);
 
diff --git a/res/res_pjsip_outbound_registration.c b/res/res_pjsip_outbound_registration.c
index 0f590f5..1b8fbe2 100644
--- a/res/res_pjsip_outbound_registration.c
+++ b/res/res_pjsip_outbound_registration.c
@@ -586,6 +586,7 @@
 
 	if (client_state->support_path) {
 		pjsip_supported_hdr *hdr;
+		int i;
 
 		hdr = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_SUPPORTED, NULL);
 		if (!hdr) {
@@ -599,6 +600,17 @@
 			pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *)hdr);
 		}
 
+		/* Don't add the value if it's already there */
+		for (i = 0; i < hdr->count; ++i) {
+			if (pj_stricmp(&hdr->values[i], &PATH_NAME) == 0) {
+				return 1;
+			}
+		}
+
+		if (hdr->count >= PJSIP_GENERIC_ARRAY_MAX_COUNT) {
+			return 0;
+		}
+
 		/* add on to the existing Supported header */
 		pj_strassign(&hdr->values[hdr->count++], &PATH_NAME);
 	}
diff --git a/res/res_pjsip_path.c b/res/res_pjsip_path.c
index de231fb..28f2cfd 100644
--- a/res/res_pjsip_path.c
+++ b/res/res_pjsip_path.c
@@ -123,6 +123,7 @@
 static int add_supported(pjsip_tx_data *tdata)
 {
 	pjsip_supported_hdr *hdr;
+	int i;
 
 	hdr = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_SUPPORTED, NULL);
 	if (!hdr) {
@@ -135,6 +136,17 @@
 		pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *)hdr);
 	}
 
+	/* Don't add the value if it's already there */
+	for (i = 0; i < hdr->count; ++i) {
+		if (pj_stricmp(&hdr->values[i], &PATH_SUPPORTED_NAME) == 0) {
+			return 0;
+		}
+	}
+
+	if (hdr->count >= PJSIP_GENERIC_ARRAY_MAX_COUNT) {
+		return -1;
+	}
+
 	/* add on to the existing Supported header */
 	pj_strassign(&hdr->values[hdr->count++], &PATH_SUPPORTED_NAME);
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: Id43704a1f1a0293e35cc7f844026f0b04f2ac322
Gerrit-Change-Number: 15454
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Ivan Poddubny <ivan.poddubny at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210218/a5d07041/attachment-0001.html>