[PATCH] osmo-msc[master]: MNCC: Add input validation

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

Harald Welte gerrit-no-reply at lists.osmocom.org
Mon Jan 22 00:50:27 UTC 2018


Review at  https://gerrit.osmocom.org/5965

MNCC: Add input validation

There appears to have been no input validation whatsoever on MNCC
messages.  Hence it was very easy for an external MNCC handler to
crash OsmoMSC, such as in OS#2853

Change-Id: Idaf3b8e409c84564b1eb26d01a19c605f89b14f4
Closes: OS#2853
---
M include/osmocom/msc/mncc.h
M src/libmsc/mncc.c
M src/libmsc/mncc_sock.c
3 files changed, 188 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/65/5965/1

diff --git a/include/osmocom/msc/mncc.h b/include/osmocom/msc/mncc.h
index 49f0c8b..d2f0541 100644
--- a/include/osmocom/msc/mncc.h
+++ b/include/osmocom/msc/mncc.h
@@ -215,5 +215,6 @@
 		|| msg_type == GSM_TCH_FRAME_AMR \
 		|| msg_type == GSM_BAD_FRAME)
 
+int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len);
 
 #endif
diff --git a/src/libmsc/mncc.c b/src/libmsc/mncc.c
index 4e88bc6..3b4c41d 100644
--- a/src/libmsc/mncc.c
+++ b/src/libmsc/mncc.c
@@ -1,7 +1,7 @@
 /* mncc.c - utility routines for the MNCC API between the 04.08
  *	    message parsing and the actual Call Control logic */
 
-/* (C) 2008-2009 by Harald Welte <laforge at gnumonks.org>
+/* (C) 2008-2018 by Harald Welte <laforge at gnumonks.org>
  * (C) 2009 by Andreas Eversberg <Andreas.Eversberg at versatel.de>
  * All Rights Reserved
  *
@@ -105,3 +105,185 @@
 	data->cause.value = val;
 }
 
+
+/***********************************************************************
+ * MNCC validation code. Move to libosmocore once headers are merged
+ ************************************************************************/
+
+#define MNCC_F_ALL 0x3fff
+
+static int check_string_terminated(const char *str, unsigned int size)
+{
+	int i;
+	for (i = 0; i < size; i++) {
+		if (str[i] == 0)
+			return 0;
+	}
+	return -EINVAL;
+}
+
+static int mncc_check_number(const struct gsm_mncc_number *num, const char *str)
+{
+	int rc;
+	rc = check_string_terminated(num->number, ARRAY_SIZE(num->number));
+	if (rc < 0)
+		LOGP(DMNCC, LOGL_ERROR, "MNCC %s number not terminated\n", str);
+	return rc;
+}
+
+static int mncc_check_cause(const struct gsm_mncc_cause *cause)
+{
+	if (cause->diag_len > sizeof(cause->diag))
+		return -EINVAL;
+	return 0;
+}
+
+static int mncc_check_useruser(const struct gsm_mncc_useruser *uu)
+{
+	return check_string_terminated(uu->info, ARRAY_SIZE(uu->info));
+}
+
+static int mncc_check_facility(const struct gsm_mncc_facility *fac)
+{
+	return check_string_terminated(fac->info, ARRAY_SIZE(fac->info));
+}
+
+static int mncc_check_ssversion(const struct gsm_mncc_ssversion *ssv)
+{
+	return check_string_terminated(ssv->info, ARRAY_SIZE(ssv->info));
+}
+
+static int mncc_prim_check_sign(const struct gsm_mncc *mncc_prim)
+{
+	int rc;
+
+	if (mncc_prim->fields & ~ MNCC_F_ALL) {
+		LOGP(DMNCC, LOGL_ERROR, "Unknown MNCC field mask 0x%x\n", mncc_prim->fields);
+		return -EINVAL;
+	}
+
+	rc = check_string_terminated(mncc_prim->imsi, sizeof(mncc_prim->imsi));
+	if (rc < 0) {
+		LOGP(DMNCC, LOGL_ERROR, "MNCC IMSI not terminated\n");
+		return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_CALLED) {
+		rc = mncc_check_number(&mncc_prim->called, "called");
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_CALLING) {
+		rc = mncc_check_number(&mncc_prim->calling, "calling");
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_REDIRECTING) {
+		rc = mncc_check_number(&mncc_prim->redirecting, "redirecting");
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_CONNECTED) {
+		rc = mncc_check_number(&mncc_prim->connected, "connected");
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_CAUSE) {
+		rc = mncc_check_cause(&mncc_prim->cause);
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_USERUSER) {
+		rc = mncc_check_useruser(&mncc_prim->useruser);
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_FACILITY) {
+		rc = mncc_check_facility(&mncc_prim->facility);
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_SSVERSION) {
+		rc = mncc_check_ssversion(&mncc_prim->ssversion);
+		if (rc < 0)
+			return rc;
+	}
+
+	if (mncc_prim->fields & MNCC_F_BEARER_CAP) {
+		bool m1_found = false;
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(mncc_prim->bearer_cap.speech_ver); i++) {
+			if (mncc_prim->bearer_cap.speech_ver[i] == -1) {
+				m1_found = true;
+				break;
+			}
+		}
+		if (!m1_found) {
+			LOGP(DMNCC, LOGL_ERROR, "Unterminated MNCC bearer capability\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+int mncc_prim_check(const struct gsm_mncc *mncc_prim, unsigned int len)
+{
+	if (len < sizeof(mncc_prim->msg_type)) {
+		LOGP(DMNCC, LOGL_ERROR, "Short MNCC Header\n");
+		return -EINVAL;
+	}
+
+	switch (mncc_prim->msg_type) {
+	case MNCC_SOCKET_HELLO:
+		if (len < sizeof(struct gsm_mncc_hello)) {
+			LOGP(DMNCC, LOGL_ERROR, "Short MNCC Hello\n");
+			return -EINVAL;
+		}
+		break;
+	case GSM_BAD_FRAME:
+	case GSM_TCH_FRAME_AMR:
+	case GSM_TCHH_FRAME:
+	case GSM_TCHF_FRAME_EFR:
+	case GSM_TCHF_FRAME:
+		if (len < sizeof(struct gsm_data_frame)) {
+			LOGP(DMNCC, LOGL_ERROR, "Short MNCC TCH\n");
+			return -EINVAL;
+		}
+		break;
+	case MNCC_RTP_FREE:
+	case MNCC_RTP_CONNECT:
+	case MNCC_RTP_CREATE:
+		if (len < sizeof(struct gsm_mncc_rtp)) {
+			LOGP(DMNCC, LOGL_ERROR, "Short MNCC RTP\n");
+			return -EINVAL;
+		}
+		break;
+	case MNCC_LCHAN_MODIFY:
+	case MNCC_FRAME_DROP:
+	case MNCC_FRAME_RECV:
+		/* FIXME */
+		break;
+	case MNCC_BRIDGE:
+		if (len < sizeof(struct gsm_mncc_bridge)) {
+			LOGP(DMNCC, LOGL_ERROR, "Short MNCC BRIDGE\n");
+			return -EINVAL;
+		}
+		break;
+	default:
+		if (len < sizeof(struct gsm_mncc)) {
+			LOGP(DMNCC, LOGL_ERROR, "Short MNCC Signalling\n");
+			return -EINVAL;
+		}
+		return mncc_prim_check_sign(mncc_prim);
+	}
+	return 0;
+}
diff --git a/src/libmsc/mncc_sock.c b/src/libmsc/mncc_sock.c
index b6b1bc9..14613ca 100644
--- a/src/libmsc/mncc_sock.c
+++ b/src/libmsc/mncc_sock.c
@@ -123,8 +123,11 @@
 			return 0;
 		goto close;
 	}
+	msgb_put(msg, rc);
 
-	rc = mncc_tx_to_cc(state->net, mncc_prim->msg_type, mncc_prim);
+	rc = mncc_prim_check(mncc_prim, rc);
+	if (rc == 0)
+		rc = mncc_tx_to_cc(state->net, mncc_prim->msg_type, mncc_prim);
 
 	/* as we always synchronously process the message in mncc_send() and
 	 * its callbacks, we can free the message here. */

-- 
To view, visit https://gerrit.osmocom.org/5965
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idaf3b8e409c84564b1eb26d01a19c605f89b14f4
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>



More information about the gerrit-log mailing list