<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/12641">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Max: Looks good to me, approved
tnt: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">constrain gsm48_generate_mid() output array bounds<br><br>The longest BCd-digit type identity is the IMEISV with 16, so there's<br>no point in trying to parse up to 255 decimal digits, which will do<br>nothing but to overflow the caller-provided output buffer.<br><br>Let's also clearly define the required minimum size of the output<br>buffer and add a reltead #define for it.<br><br>Change-Id: Ic8488bc7f77dc9182e372741b88f0f06100dddc9<br>---<br>M include/osmocom/gsm/gsm48.h<br>M src/gb/gprs_bssgp.c<br>M src/gb/gprs_bssgp_bss.c<br>M src/gsm/gsm48.c<br>4 files changed, 12 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/gsm/gsm48.h b/include/osmocom/gsm/gsm48.h</span><br><span>index 0f5727a..7e0e5c4 100644</span><br><span>--- a/include/osmocom/gsm/gsm48.h</span><br><span>+++ b/include/osmocom/gsm/gsm48.h</span><br><span>@@ -45,6 +45,7 @@</span><br><span> OSMO_DEPRECATED("Use gsm48_generate_lai2() instead, to not lose leading zeros in the MNC");</span><br><span> void gsm48_generate_lai2(struct gsm48_loc_area_id *lai48, const struct osmo_location_area_id *lai);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define GSM48_MID_MAX_SIZE 11</span><br><span> int gsm48_generate_mid_from_tmsi(uint8_t *buf, uint32_t tmsi);</span><br><span> int gsm48_generate_mid_from_imsi(uint8_t *buf, const char *imsi);</span><br><span> uint8_t gsm48_generate_mid(uint8_t *buf, const char *id, uint8_t mi_type);</span><br><span>diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c</span><br><span>index 3b9fbf9..be7ef9f 100644</span><br><span>--- a/src/gb/gprs_bssgp.c</span><br><span>+++ b/src/gb/gprs_bssgp.c</span><br><span>@@ -1156,7 +1156,7 @@</span><br><span> </span><br><span> /* IMSI */</span><br><span> if (dup->imsi && strlen(dup->imsi)) {</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi[10];</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mi[GSM48_MID_MAX_SIZE];</span><br><span> int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);</span><br><span> if (imsi_len > 2)</span><br><span> msgb_tvlv_push(msg, BSSGP_IE_IMSI,</span><br><span>@@ -1205,7 +1205,7 @@</span><br><span> struct bssgp_normal_hdr *bgph =</span><br><span> (struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));</span><br><span> uint16_t drx_params = osmo_htons(pinfo->drx_params);</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi[10];</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mi[GSM48_MID_MAX_SIZE];</span><br><span> int imsi_len = gsm48_generate_mid_from_imsi(mi, pinfo->imsi);</span><br><span> struct gsm48_ra_id ra;</span><br><span> </span><br><span>diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c</span><br><span>index 487286c..77350e2 100644</span><br><span>--- a/src/gb/gprs_bssgp_bss.c</span><br><span>+++ b/src/gb/gprs_bssgp_bss.c</span><br><span>@@ -178,7 +178,7 @@</span><br><span> const char *imsi)</span><br><span> {</span><br><span> struct msgb *msg = common_tx_radio_status(bctx);</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t mi[10];</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t mi[GSM48_MID_MAX_SIZE];</span><br><span> int imsi_len = gsm48_generate_mid_from_imsi(mi, imsi);</span><br><span> </span><br><span> if (!msg)</span><br><span>diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c</span><br><span>index 795e98b..86d40d4 100644</span><br><span>--- a/src/gsm/gsm48.c</span><br><span>+++ b/src/gsm/gsm48.c</span><br><span>@@ -637,20 +637,23 @@</span><br><span> return 7;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! Generate TS 24.008 §10.5.1.4 Mobile ID</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[out] buf Caller-provided output buffer</span><br><span style="color: hsl(120, 100%, 40%);">+/*! Generate TS 24.008 §10.5.1.4 Mobile ID of BCD type from ASCII string</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[out] buf Caller-provided output buffer of at least GSM48_MID_MAX_SIZE bytes</span><br><span> * \param[in] id Identity to be encoded</span><br><span style="color: hsl(0, 100%, 40%);">- * \param[in] mi_type Type of identity (e.g. GSM_MI_TYPE_TMSI)</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] mi_type Type of identity (e.g. GSM_MI_TYPE_IMSI, IMEI, IMEISV)</span><br><span> * \returns number of bytes used in \a buf */</span><br><span> uint8_t gsm48_generate_mid(uint8_t *buf, const char *id, uint8_t mi_type)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t length = strnlen(id, 255), i, off = 0, odd = (length & 1) == 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t length = strnlen(id, 16), i, off = 0, odd = (length & 1) == 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* maximum length == 16 (IMEISV) */</span><br><span> </span><br><span> buf[0] = GSM48_IE_MOBILE_ID;</span><br><span> buf[2] = osmo_char2bcd(id[0]) << 4 | (mi_type & GSM_MI_TYPE_MASK) | (odd << 3);</span><br><span> </span><br><span> /* if the length is even we will fill half of the last octet */</span><br><span> buf[1] = (length + (odd ? 1 : 2)) >> 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* buf[1] maximum = 18/2 = 9 */</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(buf[1] <= 9);</span><br><span> </span><br><span> for (i = 1; i < buf[1]; ++i) {</span><br><span> uint8_t upper, lower = osmo_char2bcd(id[++off]);</span><br><span>@@ -662,6 +665,7 @@</span><br><span> buf[2 + i] = (upper << 4) | lower;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* maximum return value: 2 + 9 = 11 */</span><br><span> return 2 + buf[1];</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/12641">change 12641</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/12641"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ic8488bc7f77dc9182e372741b88f0f06100dddc9 </div>
<div style="display:none"> Gerrit-Change-Number: 12641 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </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: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: tnt <tnt@246tNt.com> </div>
<div style="display:none"> Gerrit-CC: Vadim Yanitskiy <axilirator@gmail.com> </div>