Change in osmo-bsc[master]: osmo_bsc_bssap: check bssmap length field

[Harald Welte]

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

Change subject: osmo_bsc_bssap: check bssmap length field
......................................................................

osmo_bsc_bssap: check bssmap length field

At the moment the length field of the bssmap header is not parsed.
Instead the length is computed out of the known header length and the
number of bytes received. This is prone to error, lets make sure that
extranous data at the end of a message is ignored by parsing the bssmap
length correctly.

Change-Id: Idef2e783d2377a2ad1f697ea4d26491a32b3e549
Related: OS#3806
---
M src/osmo-bsc/osmo_bsc_bssap.c
1 file changed, 31 insertions(+), 1 deletion(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 85aab22..65618fd 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -1081,6 +1081,36 @@
 	return 0;
 }
 
+/* Extract and verify the length information from the BSSMAP header. */
+static unsigned int bssmap_msg_len(struct msgb *msg, unsigned int length,
+				   const struct gsm_subscriber_connection *conn)
+{
+	unsigned int expected_len;
+	unsigned int calculated_len;
+	struct bssmap_header *bssmap_header;
+
+	bssmap_header = (struct bssmap_header *)msg->l3h;
+
+	calculated_len = length - sizeof(struct bssmap_header);
+	expected_len = bssmap_header->length;
+
+	/* In case of contradictory length information, decide for the
+	 * shorter length */
+	if (calculated_len > expected_len) {
+		LOGPFSML(conn->fi, LOGL_NOTICE,
+			 "BSSMAP message contains extra data, expected %u bytes, got %u bytes, truncated\n",
+			 expected_len, calculated_len);
+		return expected_len;
+	} else if (calculated_len < expected_len) {
+		LOGPFSML(conn->fi, LOGL_NOTICE,
+			 "Short BSSMAP message, expected %u bytes, got %u bytes\n",
+			 expected_len, calculated_len);
+		return calculated_len;
+	}
+
+	return expected_len;
+}
+
 int bsc_handle_dt(struct gsm_subscriber_connection *conn,
 		  struct msgb *msg, unsigned int len)
 {
@@ -1093,7 +1123,7 @@
 	switch (msg->l3h[0]) {
 	case BSSAP_MSG_BSS_MANAGEMENT:
 		msg->l4h = &msg->l3h[sizeof(struct bssmap_header)];
-		bssmap_rcvmsg_dt1(conn, msg, len - sizeof(struct bssmap_header));
+		bssmap_rcvmsg_dt1(conn, msg, bssmap_msg_len(msg, len, conn));
 		break;
 	case BSSAP_MSG_DTAP:
 		dtap_rcvmsg(conn, msg, len);

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idef2e783d2377a2ad1f697ea4d26491a32b3e549
Gerrit-Change-Number: 13306
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190327/ba0e6bc5/attachment.htm>