Change in osmo-mgw[master]: protocol: reject illegal multiple lco options

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Wed Jun 6 11:18:23 UTC 2018


dexter has uploaded this change for review. ( https://gerrit.osmocom.org/9474


Change subject: protocol: reject illegal multiple lco options
......................................................................

protocol: reject illegal multiple lco options

At the moment osmo-mgw will accept multiple lco options. (e.g.
p:10, a:PCMU, p:10) If LCO appear multiple times, than the first
appearance of will be parsed and used, all following appearances
will be ignored. However, having multiple appearances of LCO is
illegal and affected requests should be rejected.

- make sure that multiple appearances of LCOs will be rejected

Change-Id: Iae2fddfa5f2bcfc952f8ab217b3056694e5f7812
Closes: OS#3119
---
M src/libosmo-mgcp/mgcp_protocol.c
1 file changed, 98 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/74/9474/1

diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 9fdaf18..8eaff3b 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -395,6 +395,97 @@
 	return -1;
 }
 
+/* Helper function for check_local_cx_options() to get a pointer of the next
+ * lco option identifier */
+static char *get_lco_identifier(const char *options)
+{
+	char *ptr;
+	unsigned int count = 0;
+
+	ptr = strstr(options, ":");
+	if (!ptr)
+		return NULL;
+
+	while (1) {
+		/* Endless loop protection */
+		if (count > 10000)
+			return NULL;
+		if (ptr <= options)
+			break;
+		else if (*ptr == ' ' || *ptr == ',') {
+			ptr++;
+			break;
+		}
+		ptr--;
+		count++;
+	}
+
+	return ptr;
+}
+
+/* Check the LCO option. This function checks for multiple appearence of LCO
+ * options, which is illegal */
+int check_local_cx_options(void *ctx, const char *options)
+{
+	int i;
+	char *options_copy;
+	char *lco_identifier;
+	char *lco_identifier_end;
+	char *next_lco_identifier;
+
+	char **lco_seen;
+	unsigned int lco_seen_n = 0;
+
+	if (!options)
+		return -1;
+
+	lco_seen = (char **)talloc_zero_size(ctx, strlen(options) * sizeof(char *));
+	options_copy = talloc_strdup(ctx, options);
+	lco_identifier = options_copy;
+
+	do {
+		/* Move the lco_identifier pointer to the beginning of the
+		 * current lco option identifier */
+		lco_identifier = get_lco_identifier(lco_identifier);
+		if (!lco_identifier)
+			goto error;
+
+		/* Look ahead to the next LCO option early, since we
+		 * will parse destructively */
+		next_lco_identifier = strstr(lco_identifier + 1, ",");
+
+		/* Pinch off the end of the lco field identifier name
+		 * and see if we still got something */
+		lco_identifier_end = strstr(lco_identifier, ":");
+		if (!lco_identifier_end)
+			goto error;
+		*lco_identifier_end = '\0';
+		if (strlen(lco_identifier) == 0)
+			goto error;
+
+		/* Check if we have already seen the current field identifier
+		 * before. If yes, we must bail, an LCO must only appear once
+		 * in the LCO string */
+		for (i = 0; i < lco_seen_n; i++) {
+			if (strcmp(lco_seen[i], lco_identifier) == 0)
+				goto error;
+		}
+		lco_seen[lco_seen_n] = lco_identifier;
+		lco_seen_n++;
+
+		/* Go to the next lco option */
+		lco_identifier = next_lco_identifier;
+	} while (lco_identifier);
+
+	talloc_free(lco_seen);
+	talloc_free(options_copy);
+	return 0;
+error:
+	talloc_free(lco_seen);
+	talloc_free(options_copy);
+	return -1;
+}
+
 /* Set the LCO from a string (see RFC 3435).
  * The string is stored in the 'string' field. A NULL string is handled exactly
  * like an empty string, the 'string' field is never NULL after this function
@@ -410,6 +501,13 @@
 	if (strlen(options) == 0)
 		return 0;
 
+	/* Make sure the encoding of the LCO is consistant before we proceed */
+	if (check_local_cx_options(ctx, options) != 0) {
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "local CX options: Internal inconsistency in Local Connection Options!\n");
+		return 524;
+	}
+
 	p_opt = strstr(options, "p:");
 	if (p_opt && sscanf(p_opt, "p:%d-%d",
 			    &lco->pkt_period_min, &lco->pkt_period_max) == 1)

-- 
To view, visit https://gerrit.osmocom.org/9474
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae2fddfa5f2bcfc952f8ab217b3056694e5f7812
Gerrit-Change-Number: 9474
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180606/5671923b/attachment.htm>


More information about the gerrit-log mailing list