libmsc/gsm_04_08.c: refactor CM Service Request parsing

In gsm48_rx_mm_serv_req() we need to make sure that a given message
buffer is large enough to contain both 'gsm48_hdr' and
'gsm48_service_request' structures.

Comparing msg->data_len with size of pointer if wrong because:

  - we actually need to compare with size of struct(s),
  - we need msgb_l3len(), not length of the whole buffer.

Moreover, since we have to use the pointer arithmetics in order
to keep backwards compatibility with Phase1 phones, we also
need to check the length of both Classmark2 and MI IEs.

Change-Id: I6e7454d7a6f63fd5a0e12fb90d8c58688da0951e

1 files changed, 38 insertions, 19 deletions

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 280c341d9..418ee2ab7 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -700,28 +700,47 @@ accept_reuse:
 int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
 {
 	struct gsm_network *net = msc_a_net(msc_a);
-	uint8_t mi_type;
-	struct gsm48_hdr *gh = msgb_l3(msg);
-	struct gsm48_service_request *req =
-			(struct gsm48_service_request *)gh->data;
-	/* unfortunately in Phase1 the classmark2 length is variable */
-	uint8_t classmark2_len = gh->data[1];
-	uint8_t *classmark2_buf = gh->data+2;
-	struct gsm48_classmark2 *cm2 = (void*)classmark2_buf;
-	uint8_t *mi_p = classmark2_buf + classmark2_len;
-	uint8_t mi_len = *mi_p;
-	uint8_t *mi = mi_p + 1;
+	struct gsm48_hdr *gh;
+	struct gsm48_service_request *req;
+	struct gsm48_classmark2 *cm2;
+	uint8_t *cm2_buf, cm2_len;
+	uint8_t *mi_buf, mi_len;
+	uint8_t *mi, mi_type;
 	bool is_utran;
 	struct vlr_subscr *vsub;
 
-	if (msg->data_len < sizeof(struct gsm48_service_request*)) {
+	/* Make sure that both header and CM Service Request fit into the buffer */
+	if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*req)) {
 		LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: wrong message size (%u < %zu)\n",
-			  msg->data_len, sizeof(struct gsm48_service_request*));
+			  msgb_l3len(msg), sizeof(*gh) + sizeof(*req));
 		return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);
 	}
 
-	if (msg->data_len < req->mi_len + 6) {
-		LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: message does not fit in packet\n");
+	gh = (struct gsm48_hdr *) msgb_l3(msg);
+	req = (struct gsm48_service_request *) gh->data;
+
+	/* Unfortunately in Phase1 the Classmark2 length is variable, so we cannot
+	 * just use gsm48_service_request struct, and need to parse it manually. */
+	cm2_len = gh->data[1];
+	cm2_buf = gh->data + 2;
+	cm2 = (struct gsm48_classmark2 *) cm2_buf;
+
+	/* Prevent buffer overrun: check the length of Classmark2 */
+	if (cm2_buf + cm2_len > msg->tail) {
+		LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: Classmark2 "
+					     "length=%u is too big\n", cm2_len);
+		return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);
+	}
+
+	/* MI (Mobile Identity) LV follows the Classmark2 */
+	mi_buf = cm2_buf + cm2_len;
+	mi_len = mi_buf[0];
+	mi = mi_buf + 1;
+
+	/* Prevent buffer overrun: check the length of MI */
+	if (mi_buf + mi_len > msg->tail) {
+		LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: Mobile Identity "
+					     "length=%u is too big\n", cm2_len);
 		return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_INCORRECT_MESSAGE);
 	}
 
@@ -759,9 +778,9 @@ int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
 		return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_SRV_OPT_NOT_SUPPORTED);
 
 	if (msc_a_is_accepted(msc_a))
-		return cm_serv_reuse_conn(msc_a, mi_p, req->cm_service_type);
+		return cm_serv_reuse_conn(msc_a, mi_buf, req->cm_service_type);
 
-	osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, mi_p);
+	osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, mi_buf);
 
 	msc_a_get(msc_a, msc_a_cm_service_type_to_use(req->cm_service_type));
 
@@ -775,7 +794,7 @@ int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
 			 is_utran || net->authentication_required,
 			 is_utran || net->a5_encryption_mask > 0x01,
 			 req->cipher_key_seq,
-			 osmo_gsm48_classmark2_is_r99(cm2, classmark2_len),
+			 osmo_gsm48_classmark2_is_r99(cm2, cm2_len),
 			 is_utran);
 
 	/* From vlr_proc_acc_req() we expect an implicit dispatch of PR_ARQ_E_START we expect
@@ -787,7 +806,7 @@ int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
 	}
 
 	vsub->classmark.classmark2 = *cm2;
-	vsub->classmark.classmark2_len = classmark2_len;
+	vsub->classmark.classmark2_len = cm2_len;
 	return 0;
 }