Change in libosmocore[master]: constrain gsm48_generate_mid() output array bounds

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/12641 )

Change subject: constrain gsm48_generate_mid() output array bounds
......................................................................

constrain gsm48_generate_mid() output array bounds

The longest BCd-digit type identity is the IMEISV with 16, so there's
no point in trying to parse up to 255 decimal digits, which will do
nothing but to overflow the caller-provided output buffer.

Let's also clearly define the required minimum size of the output
buffer and add a reltead #define for it.

Change-Id: Ic8488bc7f77dc9182e372741b88f0f06100dddc9
---
M include/osmocom/gsm/gsm48.h
M src/gb/gprs_bssgp.c
M src/gb/gprs_bssgp_bss.c
M src/gsm/gsm48.c
4 files changed, 12 insertions(+), 7 deletions(-)

Approvals:
  Max: Looks good to me, approved
  tnt: Looks good to me, but someone else must approve
  Jenkins Builder: Verified



diff --git a/include/osmocom/gsm/gsm48.h b/include/osmocom/gsm/gsm48.h
index 0f5727a..7e0e5c4 100644
--- a/include/osmocom/gsm/gsm48.h
+++ b/include/osmocom/gsm/gsm48.h
@@ -45,6 +45,7 @@
 	OSMO_DEPRECATED("Use gsm48_generate_lai2() instead, to not lose leading zeros in the MNC");
 void gsm48_generate_lai2(struct gsm48_loc_area_id *lai48, const struct osmo_location_area_id *lai);
 
+#define GSM48_MID_MAX_SIZE	11
 int gsm48_generate_mid_from_tmsi(uint8_t *buf, uint32_t tmsi);
 int gsm48_generate_mid_from_imsi(uint8_t *buf, const char *imsi);
 uint8_t gsm48_generate_mid(uint8_t *buf, const char *id, uint8_t mi_type);
diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c
index 3b9fbf9..be7ef9f 100644
--- a/src/gb/gprs_bssgp.c
+++ b/src/gb/gprs_bssgp.c
@@ -1156,7 +1156,7 @@
 
 	/* IMSI */
 	if (dup->imsi && strlen(dup->imsi)) {
-		uint8_t mi[10];
+		uint8_t mi[GSM48_MID_MAX_SIZE];
 		int imsi_len = gsm48_generate_mid_from_imsi(mi, dup->imsi);
 		if (imsi_len > 2)
 			msgb_tvlv_push(msg, BSSGP_IE_IMSI,
@@ -1205,7 +1205,7 @@
 	struct bssgp_normal_hdr *bgph =
 			(struct bssgp_normal_hdr *) msgb_put(msg, sizeof(*bgph));
 	uint16_t drx_params = osmo_htons(pinfo->drx_params);
-	uint8_t mi[10];
+	uint8_t mi[GSM48_MID_MAX_SIZE];
 	int imsi_len = gsm48_generate_mid_from_imsi(mi, pinfo->imsi);
 	struct gsm48_ra_id ra;
 
diff --git a/src/gb/gprs_bssgp_bss.c b/src/gb/gprs_bssgp_bss.c
index 487286c..77350e2 100644
--- a/src/gb/gprs_bssgp_bss.c
+++ b/src/gb/gprs_bssgp_bss.c
@@ -178,7 +178,7 @@
 				const char *imsi)
 {
 	struct msgb *msg = common_tx_radio_status(bctx);
-	uint8_t mi[10];
+	uint8_t mi[GSM48_MID_MAX_SIZE];
 	int imsi_len = gsm48_generate_mid_from_imsi(mi, imsi);
 
 	if (!msg)
diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c
index 795e98b..86d40d4 100644
--- a/src/gsm/gsm48.c
+++ b/src/gsm/gsm48.c
@@ -637,20 +637,23 @@
 	return 7;
 }
 
-/*! Generate TS 24.008 §10.5.1.4 Mobile ID
- *  \param[out] buf Caller-provided output buffer
+/*! Generate TS 24.008 §10.5.1.4 Mobile ID of BCD type from ASCII string
+ *  \param[out] buf Caller-provided output buffer of at least GSM48_MID_MAX_SIZE bytes
  *  \param[in] id Identity to be encoded
- *  \param[in] mi_type Type of identity (e.g. GSM_MI_TYPE_TMSI)
+ *  \param[in] mi_type Type of identity (e.g. GSM_MI_TYPE_IMSI, IMEI, IMEISV)
  *  \returns number of bytes used in \a buf */
 uint8_t gsm48_generate_mid(uint8_t *buf, const char *id, uint8_t mi_type)
 {
-	uint8_t length = strnlen(id, 255), i, off = 0, odd = (length & 1) == 1;
+	uint8_t length = strnlen(id, 16), i, off = 0, odd = (length & 1) == 1;
+	/* maximum length == 16 (IMEISV) */
 
 	buf[0] = GSM48_IE_MOBILE_ID;
 	buf[2] = osmo_char2bcd(id[0]) << 4 | (mi_type & GSM_MI_TYPE_MASK) | (odd << 3);
 
 	/* if the length is even we will fill half of the last octet */
 	buf[1] = (length + (odd ? 1 : 2)) >> 1;
+	/* buf[1] maximum = 18/2 = 9 */
+	OSMO_ASSERT(buf[1] <= 9);
 
 	for (i = 1; i < buf[1]; ++i) {
 		uint8_t upper, lower = osmo_char2bcd(id[++off]);
@@ -662,6 +665,7 @@
 		buf[2 + i] = (upper << 4) | lower;
 	}
 
+	/* maximum return value: 2 + 9 = 11 */
 	return 2 + buf[1];
 }
 

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic8488bc7f77dc9182e372741b88f0f06100dddc9
Gerrit-Change-Number: 12641
Gerrit-PatchSet: 4
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: tnt <tnt at 246tNt.com>
Gerrit-CC: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190122/046df374/attachment.htm>