<p>Neels Hofmeyr <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/10754">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">fix mgcp_verify_ci(): off-by-one in max len check<br><br>MGCP_CONN_ID_MAXLEN actually includes a terminating nul, so we need to compare<br>strlen() against MGCP_CONN_ID_MAXLEN-1.<br><br>Log the length if it is too long.<br><br>Add MDCX_TOO_LONG_CI test to mgcp_test.c, testing a conn id of 33 characters.<br>Before this patch, the test returns error code 515 meaning "not found", while<br>now it returns 510 meaning "invalid", showing the off-by-one. Same is<br>illustrated by the error log ("not found" before, "too long" now), but the<br>error log is not verified by mgcp_test.c.<br><br>Change-Id: I8d6cc96be252bb486e94f343a8c7cae641ff9429<br>---<br>M src/libosmo-mgcp/mgcp_msg.c<br>M tests/mgcp/mgcp_test.c<br>M tests/mgcp/mgcp_test.ok<br>3 files changed, 23 insertions(+), 3 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c</span><br><span>index f732158..648d86b 100644</span><br><span>--- a/src/libosmo-mgcp/mgcp_msg.c</span><br><span>+++ b/src/libosmo-mgcp/mgcp_msg.c</span><br><span>@@ -454,10 +454,10 @@</span><br><span> }</span><br><span> </span><br><span> /* Check for over long connection identifiers */</span><br><span style="color: hsl(0, 100%, 40%);">- if (strlen(conn_id) > MGCP_CONN_ID_MAXLEN) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (strlen(conn_id) > (MGCP_CONN_ID_MAXLEN-1)) {</span><br><span> LOGP(DLMGCP, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "endpoint:0x%x invalid ConnectionIdentifier (too long) 0x%s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- ENDPOINT_NUMBER(endp), conn_id);</span><br><span style="color: hsl(120, 100%, 40%);">+ "endpoint:0x%x invalid ConnectionIdentifier (too long: %zu > %d) 0x%s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ ENDPOINT_NUMBER(endp), strlen(conn_id), MGCP_CONN_ID_MAXLEN-1, conn_id);</span><br><span> return 510;</span><br><span> }</span><br><span> </span><br><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index 2f3a8a7..c40eabc 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -230,6 +230,12 @@</span><br><span> "I: %s\r\n" \</span><br><span> "L: p:20, a:AMR, nt:IN\r\n"</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define MDCX_TOO_LONG_CI \</span><br><span style="color: hsl(120, 100%, 40%);">+ "MDCX 18983222 1@mgw MGCP 1.0\r\n" \</span><br><span style="color: hsl(120, 100%, 40%);">+ "I: 123456789012345678901234567890123\n"</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#define MDCX_TOO_LONG_CI_RET "510 18983222 FAIL\r\n"</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #define SHORT2 "CRCX 1"</span><br><span> #define SHORT2_RET "510 000000 FAIL\r\n"</span><br><span> #define SHORT3 "CRCX 1 1@mgw"</span><br><span>@@ -510,6 +516,7 @@</span><br><span> {"DLCX", DLCX, DLCX_RET, PTYPE_IGNORE,.extra_fmtp = "a=fmtp:126 0/1/2"},</span><br><span> {"CRCX", CRCX_NO_LCO_NO_SDP, CRCX_NO_LCO_NO_SDP_RET, 97},</span><br><span> {"CRCX", CRCX_X_OSMO_IGN, CRCX_X_OSMO_IGN_RET, 97},</span><br><span style="color: hsl(120, 100%, 40%);">+ {"MDCX_TOO_LONG_CI", MDCX_TOO_LONG_CI, MDCX_TOO_LONG_CI_RET},</span><br><span> };</span><br><span> </span><br><span> static const struct mgcp_test retransmit[] = {</span><br><span>diff --git a/tests/mgcp/mgcp_test.ok b/tests/mgcp/mgcp_test.ok</span><br><span>index 915d45e..ddda751 100644</span><br><span>--- a/tests/mgcp/mgcp_test.ok</span><br><span>+++ b/tests/mgcp/mgcp_test.ok</span><br><span>@@ -443,6 +443,19 @@</span><br><span> Dummy packets: 2</span><br><span> </span><br><span> ================================================</span><br><span style="color: hsl(120, 100%, 40%);">+Testing MDCX_TOO_LONG_CI</span><br><span style="color: hsl(120, 100%, 40%);">+creating message from statically defined input:</span><br><span style="color: hsl(120, 100%, 40%);">+---------8<---------</span><br><span style="color: hsl(120, 100%, 40%);">+MDCX 18983222 1@mgw MGCP 1.0
</span><br><span style="color: hsl(120, 100%, 40%);">+I: 123456789012345678901234567890123</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+---------8<---------</span><br><span style="color: hsl(120, 100%, 40%);">+checking response:</span><br><span style="color: hsl(120, 100%, 40%);">+using message as statically defined for comparison</span><br><span style="color: hsl(120, 100%, 40%);">+Response matches our expectations.</span><br><span style="color: hsl(120, 100%, 40%);">+(response does not contain a connection id)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+================================================</span><br><span> Testing CRCX</span><br><span> creating message from statically defined input:</span><br><span> ---------8<---------</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/10754">change 10754</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/10754"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I8d6cc96be252bb486e94f343a8c7cae641ff9429 </div>
<div style="display:none"> Gerrit-Change-Number: 10754 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>