[asterisk-commits] res format attr opus: Fix crash when fmtp contains spaces. (asterisk[13])

[SVN commits to the Asterisk project]

Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/4578 )

Change subject: res_format_attr_opus: Fix crash when fmtp contains spaces.
......................................................................


res_format_attr_opus: Fix crash when fmtp contains spaces.

When an opus offer or answer was received that contained an
fmtp line with spaces between the attributes the module would
fail to properly parse it and crash due to recursion.

This change makes the module handle the space properly and
also removes the recursion requirement.

ASTERISK-26579

Change-Id: I01f53e5d9fa9f1925a7365f8d25071b5b3ac2dc3
---
M res/res_format_attr_opus.c
1 file changed, 23 insertions(+), 15 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, approved
  Mark Michelson: Looks good to me, but someone else must approve
  Joshua Colp: Verified



diff --git a/res/res_format_attr_opus.c b/res/res_format_attr_opus.c
index 49382f6..857b943 100644
--- a/res/res_format_attr_opus.c
+++ b/res/res_format_attr_opus.c
@@ -102,27 +102,35 @@
 
 static void sdp_fmtp_get(const char *attributes, const char *name, int *attr)
 {
-	const char *kvp = "";
+	const char *kvp = attributes;
 	int val;
 
-	if (attributes && !(kvp = strstr(attributes, name))) {
+	if (ast_strlen_zero(attributes)) {
 		return;
 	}
 
-	/*
-	 * If the named attribute is not at the start of the given attributes, and
-	 * the preceding character is not a space or semicolon then it's not the
-	 * attribute we are looking for. It's an attribute with the name embedded
-	 * within it (e.g. ptime in maxptime, stereo in sprop-stereo).
+	/* This logic goes through each attribute in the fmtp line looking for the
+	 * requested named attribute.
 	 */
-	if (kvp != attributes && *(kvp - 1) != ' ' && *(kvp - 1) != ';') {
-		/* Keep searching as it might still be in the attributes string */
-		sdp_fmtp_get(strchr(kvp, ';'), name, attr);
-	/*
-	 * Otherwise it's a match, so retrieve the value and set the attribute.
-	 */
-	} else if (sscanf(kvp, "%*[^=]=%30d", &val) == 1) {
-		*attr = val;
+	while (*kvp) {
+		/* Skip any preceeding blanks as some implementations separate attributes using spaces too */
+		kvp = ast_skip_blanks(kvp);
+
+		/* If we are at at the requested attribute get its value and return */
+		if (!strncmp(kvp, name, strlen(name)) && kvp[strlen(name)] == '=') {
+			if (sscanf(kvp, "%*[^=]=%30d", &val) == 1) {
+				*attr = val;
+				break;
+			}
+		}
+
+		/* Move on to the next attribute if possible */
+		kvp = strchr(kvp, ';');
+		if (!kvp) {
+			break;
+		}
+
+		kvp++;
 	}
 }
 

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I01f53e5d9fa9f1925a7365f8d25071b5b3ac2dc3
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>