<p>Vadim Yanitskiy has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/13655">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">libmsc/gsm_04_11.c: properly handle TP-User-Data-Length<br><br>As per 3GPP TS 03.40, section 9.2.3.16 "TP-User-Data-Length (TP-UDL)",<br>if the TP-User-Data is coded using the GSM 7-bit default alphabet,<br>the TP-User-Data-Length field indicates the *number of septets*<br>within the TP-User-Data field to follow. Otherwise, i.e. in case<br>of 8-bit or UCS-2 encoded data, the *number of octets* is indicated.<br><br>Since we store the original TP-UDL value (as received), we might<br>need to convert septets to octets before passing it to memcpy().<br>Otherwise this would lead to a buffer overrun.<br><br>Also, as we receive TPDU from untrusted source (i.e. subscriber),<br>the TP-UDL value needs to be checked against the corresponding<br>maximum (160 septets or 140 octets) and truncated if needed.<br><br>Please note that buffer overrun is still possible, e.g. when an<br>indicated TP-UDL value is grather than the remaining TPDU length.<br>Preventing this would require adding an additional check.<br><br>Change-Id: I4b08db7665e854a045129e7695e2bdf296df1688<br>Depends-on: (core) I54f88d2908ac47228813fb8c049f4264e5145241<br>---<br>M src/libmsc/gsm_04_11.c<br>1 file changed, 29 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/55/13655/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/libmsc/gsm_04_11.c b/src/libmsc/gsm_04_11.c</span><br><span>index 434f878..6eea662 100644</span><br><span>--- a/src/libmsc/gsm_04_11.c</span><br><span>+++ b/src/libmsc/gsm_04_11.c</span><br><span>@@ -42,6 +42,7 @@</span><br><span> #include <osmocom/gsm/gsm_utils.h></span><br><span> #include <osmocom/gsm/gsm0411_utils.h></span><br><span> #include <osmocom/gsm/protocol/gsm_04_11.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/gsm/protocol/gsm_03_40.h></span><br><span> </span><br><span> #include <osmocom/msc/debug.h></span><br><span> #include <osmocom/msc/gsm_data.h></span><br><span>@@ -559,19 +560,35 @@</span><br><span> rc = GSM411_RP_CAUSE_MO_NET_OUT_OF_ORDER;</span><br><span> goto out;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- gsms->user_data_len = *smsp++;</span><br><span style="color: hsl(0, 100%, 40%);">- if (gsms->user_data_len) {</span><br><span style="color: hsl(0, 100%, 40%);">- memcpy(gsms->user_data, smsp, gsms->user_data_len);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- switch (sms_alphabet) {</span><br><span style="color: hsl(0, 100%, 40%);">- case DCS_7BIT_DEFAULT:</span><br><span style="color: hsl(0, 100%, 40%);">- gsm_7bit_decode_n(gsms->text, sizeof(gsms->text), smsp,</span><br><span style="color: hsl(0, 100%, 40%);">- gsms->user_data_len);</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- case DCS_8BIT_DATA:</span><br><span style="color: hsl(0, 100%, 40%);">- case DCS_UCS2:</span><br><span style="color: hsl(0, 100%, 40%);">- case DCS_NONE:</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* As per 3GPP TS 03.40, section 9.2.3.16, TP-User-Data-Length (TP-UDL)</span><br><span style="color: hsl(120, 100%, 40%);">+ * may indicate either the number of septets, or the number of octets,</span><br><span style="color: hsl(120, 100%, 40%);">+ * depending on Data Coding Scheme. We store TP-UDL value as-is,</span><br><span style="color: hsl(120, 100%, 40%);">+ * so this should be kept in mind to avoid buffer overruns. */</span><br><span style="color: hsl(120, 100%, 40%);">+ gsms->user_data_len = *smsp++;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsms->user_data_len > 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (sms_alphabet == DCS_7BIT_DEFAULT) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TP-UDL is indicated in septets (up to 160) */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsms->user_data_len > GSM340_UDL_SPT_MAX) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_TRANS(trans, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "TP-User-Data-Length %u (septets) "</span><br><span style="color: hsl(120, 100%, 40%);">+ "is too big, truncating to %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsms->user_data_len, GSM340_UDL_SPT_MAX);</span><br><span style="color: hsl(120, 100%, 40%);">+ gsms->user_data_len = GSM340_UDL_SPT_MAX;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ memcpy(gsms->user_data, smsp, gsm_get_octet_len(gsms->user_data_len));</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_7bit_decode_n(gsms->text, sizeof(gsms->text),</span><br><span style="color: hsl(120, 100%, 40%);">+ smsp, gsms->user_data_len);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* TP-UDL is indicated in octets (up to 140) */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (gsms->user_data_len > GSM340_UDL_OCT_MAX) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_TRANS(trans, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "TP-User-Data-Length %u (octets) "</span><br><span style="color: hsl(120, 100%, 40%);">+ "is too big, truncating to %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsms->user_data_len, GSM340_UDL_OCT_MAX);</span><br><span style="color: hsl(120, 100%, 40%);">+ gsms->user_data_len = GSM340_UDL_OCT_MAX;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ memcpy(gsms->user_data, smsp, gsms->user_data_len);</span><br><span> }</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13655">change 13655</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/13655"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I4b08db7665e854a045129e7695e2bdf296df1688 </div>
<div style="display:none"> Gerrit-Change-Number: 13655 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>