Change in osmo-msc[master]: libmsc/gsm_04_08.c: fix: verify MI before calling vlr_subscr_rx_id_re...

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
Sun Jan 5 11:23:09 UTC 2020


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/16683 )

Change subject: libmsc/gsm_04_08.c: fix: verify MI before calling vlr_subscr_rx_id_resp()
......................................................................

libmsc/gsm_04_08.c: fix: verify MI before calling vlr_subscr_rx_id_resp()

During the last congress, we have noticed that OsmoMSC crashes
on receipt of malformed MM Identity Response messages:

  BSSAP
      Message Type: Direct Transfer (0x01)
      Data Link Connection Identifier
          00.. .... = Control Channel: not further specified (0x0)
          ..00 0... = Spare: 0x0
          .... .000 = SAPI: RR/MM/CC (0x0)
      Length: 11
  GSM A-I/F DTAP - Identity Response
      Protocol Discriminator: Mobility Management messages (5)
          .... 0101 = Protocol discriminator: Mobility Management messages (0x5)
          0000 .... = Skip Indicator: No indication of selected PLMN (0)
      01.. .... = Sequence number: 1
      ..01 1001 = DTAP Mobility Management Message Type: Identity Response (0x19)
      Mobile Identity - Format Unknown
          Length: 8
          .... 1... = Odd/even indication: Odd number of identity digits
          .... .111 = Mobile Identity Type: Unknown (7)  <-- This makes OsmoMSC crash
              [Expert Info (Warning/Protocol): Unknown format 7]
                  [Unknown format 7]
                  [Severity level: Warning]
                  [Group: Protocol]

The value '111'B is not a valid Mobile Identity type, and shall be
considered as reserved according to 3GPP TS 24.008, section 10.5.1.4.
Later on it was discovered that '000'B also crashes OsmoMSC in the same way.

The crash itself is provoked by OSMO_ASSERT(0) in vlr_subscr_rx_id_resp().
Let's keep that assert in there, and make sure that:

  - on receipt of MM Identity Response, Mobile Identity type
    matches the one in MM Identity Request;

  - on receipt of RR Ciphering Mode Complete, Mobile Identity
    contains IMEI(SV) if present.

Change-Id: Ica4c90b8eb4d90325313c6eb400fa4a6bc5df825
TTCN-3 test case: I62f23355eb91df2edf9dc837c928cb86b530b743
Fixes: OS#4340
---
M include/osmocom/msc/msc_a.h
M src/libmsc/gsm_04_08.c
2 files changed, 39 insertions(+), 0 deletions(-)

Approvals:
  laforge: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/osmocom/msc/msc_a.h b/include/osmocom/msc/msc_a.h
index a4d3226..18973f9 100644
--- a/include/osmocom/msc/msc_a.h
+++ b/include/osmocom/msc/msc_a.h
@@ -99,6 +99,9 @@
 	/* After Ciphering Mode Complete on GERAN, this reflects the chosen ciphering algorithm and key */
 	struct geran_encr geran_encr;
 
+	/* Type of MI requested in MM Identity Request */
+	uint8_t mm_id_req_type;
+
 	/* N(SD) expected in the received frame, per flow (TS 24.007 11.2.3.2.3.2.2) */
 	uint8_t n_sd_next[4];
 
diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 750c766..b284ccd 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -182,6 +182,7 @@
 	struct gsm48_hdr *gh = msgb_l3(msg);
 	uint8_t *mi = gh->data+1;
 	uint8_t mi_len = gh->data[0];
+	uint8_t mi_type;
 	struct vlr_subscr *vsub = msc_a_vsub(msc_a);
 
 	if (!vsub) {
@@ -190,6 +191,28 @@
 		return -EINVAL;
 	}
 
+	/* There muct be at least one octet with MI type */
+	if (!mi_len) {
+		LOGP(DMM, LOGL_NOTICE, "MM Identity Response contains "
+				       "malformed Mobile Identity\n");
+		return -EINVAL;
+	}
+
+	/* Make sure we got what we expected */
+	mi_type = mi[0] & GSM_MI_TYPE_MASK;
+	if (mi_type == GSM_MI_TYPE_NONE) {
+		LOGP(DMM, LOGL_NOTICE, "MM Identity Response contains no identity, "
+				       "perhaps the MS has no Mobile Identity type %s?\n",
+				       gsm48_mi_type_name(msc_a->mm_id_req_type));
+		return -EINVAL;
+	} else if (mi_type != msc_a->mm_id_req_type) {
+		LOGP(DMM, LOGL_NOTICE, "MM Identity Response contains unexpected "
+				       "Mobile Identity type %s (extected %s)\n",
+				       gsm48_mi_type_name(mi_type),
+				       gsm48_mi_type_name(msc_a->mm_id_req_type));
+		return -EINVAL;
+	}
+
 	DEBUGP(DMM, "IDENTITY RESPONSE: MI=%s\n", osmo_mi_name(mi, mi_len));
 
 	osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, gh->data);
@@ -1182,8 +1205,17 @@
 	tlv_parse(&tp, &gsm48_att_tlvdef, gh->data, payload_len, 0, 0);
 	mi = TLVP_GET(&tp, GSM48_IE_MOBILE_ID);
 
+	/* IMEI(SV) is optional for this message */
 	if (!mi)
 		return 0;
+	if (!mi->len)
+		return -EINVAL;
+	if ((mi->val[0] & GSM_MI_TYPE_MASK) != GSM_MI_TYPE_IMEISV) {
+		LOGP(DMM, LOGL_ERROR, "RR Ciphering Mode Complete contains "
+				      "unexpected Mobile Identity type %s\n",
+				      gsm48_mi_type_name(mi->val[0] & GSM_MI_TYPE_MASK));
+		return -EINVAL;
+	}
 
 	LOG_MSC_A(msc_a, LOGL_DEBUG, "RR Ciphering Mode Complete contains Mobile Identity: %s\n",
 		  osmo_mi_name(mi->val, mi->len));
@@ -1287,6 +1319,10 @@
 static int msc_vlr_tx_id_req(void *msc_conn_ref, uint8_t mi_type)
 {
 	struct msc_a *msc_a = msc_conn_ref;
+
+	/* Store requested MI type, so we can check the response */
+	msc_a->mm_id_req_type = mi_type;
+
 	return mm_tx_identity_req(msc_a, mi_type);
 }
 

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

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ica4c90b8eb4d90325313c6eb400fa4a6bc5df825
Gerrit-Change-Number: 16683
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <axilirator at gmail.com>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200105/778eb5be/attachment.htm>


More information about the gerrit-log mailing list