Change in osmo-mgw[master]: fix mgcp_verify_ci(): off-by-one in max len check

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Mon Sep 3 21:13:46 UTC 2018


Neels Hofmeyr has uploaded this change for review. ( https://gerrit.osmocom.org/10754


Change subject: fix mgcp_verify_ci(): off-by-one in max len check
......................................................................

fix mgcp_verify_ci(): off-by-one in max len check

MGCP_CONN_ID_MAXLEN actually includes a terminating nul, so we need to compare
strlen() against MGCP_CONN_ID_MAXLEN-1.

Log the length if it is too long.

Add MDCX_TOO_LONG_CI test to mgcp_test.c, testing a conn id of 33 characters.
Before this patch, the test returns error code 515 meaning "not found", while
now it returns 510 meaning "invalid", showing the off-by-one. Same is
illustrated by the error log ("not found" before, "too long" now), but the
error log is not verified by mgcp_test.c.

Change-Id: I8d6cc96be252bb486e94f343a8c7cae641ff9429
---
M src/libosmo-mgcp/mgcp_msg.c
M tests/mgcp/mgcp_test.c
M tests/mgcp/mgcp_test.ok
3 files changed, 23 insertions(+), 3 deletions(-)



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

diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index f732158..648d86b 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -454,10 +454,10 @@
 	}
 
 	/* Check for over long connection identifiers */
-	if (strlen(conn_id) > MGCP_CONN_ID_MAXLEN) {
+	if (strlen(conn_id) > (MGCP_CONN_ID_MAXLEN-1)) {
 		LOGP(DLMGCP, LOGL_ERROR,
-		     "endpoint:0x%x invalid ConnectionIdentifier (too long) 0x%s\n",
-		     ENDPOINT_NUMBER(endp), conn_id);
+		     "endpoint:0x%x invalid ConnectionIdentifier (too long: %zu > %d) 0x%s\n",
+		     ENDPOINT_NUMBER(endp), strlen(conn_id), MGCP_CONN_ID_MAXLEN-1, conn_id);
 		return 510;
 	}
 
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 2f3a8a7..c40eabc 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -230,6 +230,12 @@
 	"I: %s\r\n" \
 	"L: p:20, a:AMR, nt:IN\r\n"
 
+#define MDCX_TOO_LONG_CI \
+	"MDCX 18983222 1 at mgw MGCP 1.0\r\n" \
+	"I: 123456789012345678901234567890123\n"
+
+#define MDCX_TOO_LONG_CI_RET "510 18983222 FAIL\r\n"
+
 #define SHORT2	"CRCX 1"
 #define SHORT2_RET "510 000000 FAIL\r\n"
 #define SHORT3	"CRCX 1 1 at mgw"
@@ -510,6 +516,7 @@
 	{"DLCX", DLCX, DLCX_RET, PTYPE_IGNORE,.extra_fmtp = "a=fmtp:126 0/1/2"},
 	{"CRCX", CRCX_NO_LCO_NO_SDP, CRCX_NO_LCO_NO_SDP_RET, 97},
 	{"CRCX", CRCX_X_OSMO_IGN, CRCX_X_OSMO_IGN_RET, 97},
+	{"MDCX_TOO_LONG_CI", MDCX_TOO_LONG_CI, MDCX_TOO_LONG_CI_RET},
 };
 
 static const struct mgcp_test retransmit[] = {
diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok
index 915d45e..ddda751 100644
--- a/tests/mgcp/mgcp_test.ok
+++ b/tests/mgcp/mgcp_test.ok
@@ -443,6 +443,19 @@
 Dummy packets: 2
 
 ================================================
+Testing MDCX_TOO_LONG_CI
+creating message from statically defined input:
+---------8<---------
+MDCX 18983222 1 at mgw MGCP 1.0
+I: 123456789012345678901234567890123
+
+---------8<---------
+checking response:
+using message as statically defined for comparison
+Response matches our expectations.
+(response does not contain a connection id)
+
+================================================
 Testing CRCX
 creating message from statically defined input:
 ---------8<---------

-- 
To view, visit https://gerrit.osmocom.org/10754
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: I8d6cc96be252bb486e94f343a8c7cae641ff9429
Gerrit-Change-Number: 10754
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180903/63dc8a65/attachment.htm>


More information about the gerrit-log mailing list