Change in ...osmo-hlr[master]: rx_check_imei_req(): fix IMEI bounds checking

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

laforge gerrit-no-reply at lists.osmocom.org
Thu Jun 13 15:34:36 UTC 2019


laforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-hlr/+/14399 )

Change subject: rx_check_imei_req(): fix IMEI bounds checking
......................................................................

rx_check_imei_req(): fix IMEI bounds checking

IMEIs (without the checksum) always have 14 digits. Replace the previous
check (length <= 14) with a proper one (length == 14) and set the buffer
to the right size. While at it, add the return code of
gsm48_decode_bc_number2() to the error log message.

I have tested with new TTCN3 tests, that the length check is working
properly now.

Related: OS#2541
Change-Id: I060a8db98fb882e4815d1709a5d85bc0143a73a6
---
M src/hlr.c
1 file changed, 17 insertions(+), 7 deletions(-)

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



diff --git a/src/hlr.c b/src/hlr.c
index 33d2828..90cbac4 100644
--- a/src/hlr.c
+++ b/src/hlr.c
@@ -477,18 +477,28 @@
 {
 	struct osmo_gsup_message gsup_reply = {0};
 	struct msgb *msg_out;
-	char imei[GSM23003_IMEI_NUM_DIGITS+1] = {0};
+	char imei[GSM23003_IMEI_NUM_DIGITS_NO_CHK+1] = {0};
+	int rc;
 
-	/* Encoded IMEI length check */
-	if (!gsup->imei_enc || gsup->imei_enc_len < 1 || gsup->imei_enc[0] >= sizeof(imei)) {
-		LOGP(DMAIN, LOGL_ERROR, "%s: wrong encoded IMEI length\n", gsup->imsi);
+	/* Require IMEI */
+	if (!gsup->imei_enc) {
+		LOGP(DMAIN, LOGL_ERROR, "%s: missing IMEI\n", gsup->imsi);
 		gsup_send_err_reply(conn, gsup->imsi, gsup->message_type, GMM_CAUSE_INV_MAND_INFO);
 		return -1;
 	}
 
-	/* Decode IMEI */
-	if (gsm48_decode_bcd_number2(imei, sizeof(imei), gsup->imei_enc, gsup->imei_enc_len, 0) < 0) {
-		LOGP(DMAIN, LOGL_ERROR, "%s: failed to decode IMEI\n", gsup->imsi);
+	/* Decode IMEI (fails if IMEI is too long) */
+	rc = gsm48_decode_bcd_number2(imei, sizeof(imei), gsup->imei_enc, gsup->imei_enc_len, 0);
+	if (rc < 0) {
+		LOGP(DMAIN, LOGL_ERROR, "%s: failed to decode IMEI (rc: %i)\n", gsup->imsi, rc);
+		gsup_send_err_reply(conn, gsup->imsi, gsup->message_type, GMM_CAUSE_INV_MAND_INFO);
+		return -1;
+	}
+
+	/* Check if IMEI is too short */
+	if (strlen(imei) != GSM23003_IMEI_NUM_DIGITS_NO_CHK) {
+		LOGP(DMAIN, LOGL_ERROR, "%s: wrong encoded IMEI length (IMEI: '%s', %lu, %i)\n", gsup->imsi, imei,
+		     strlen(imei), GSM23003_IMEI_NUM_DIGITS_NO_CHK);
 		gsup_send_err_reply(conn, gsup->imsi, gsup->message_type, GMM_CAUSE_INV_MAND_INFO);
 		return -1;
 	}

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/14399
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Change-Id: I060a8db98fb882e4815d1709a5d85bc0143a73a6
Gerrit-Change-Number: 14399
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190613/55979ae8/attachment.htm>


More information about the gerrit-log mailing list