[MERGED] osmo-bsc[master]: Check and handle SMS encoding failure

[Max]

Max has submitted this change and it was merged.

Change subject: Check and handle SMS encoding failure
......................................................................


Check and handle SMS encoding failure

The SMS address encoding can fail due to gsm48_encode_bcd_number() which
was not checked for because wrong type was used. Fix this by using
correct type, checking for error and propagating it to the caller.

Change-Id: I9fc16e24f7df5ebad6f4f1b389b2c5e861be95d7
Fixes: CID57882
---
M src/osmo-bsc_nat/bsc_nat_rewrite.c
1 file changed, 22 insertions(+), 8 deletions(-)

Approvals:
  daniel: Looks good to me, but someone else must approve
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/osmo-bsc_nat/bsc_nat_rewrite.c b/src/osmo-bsc_nat/bsc_nat_rewrite.c
index 75ed4ec..7db9234 100644
--- a/src/osmo-bsc_nat/bsc_nat_rewrite.c
+++ b/src/osmo-bsc_nat/bsc_nat_rewrite.c
@@ -340,10 +340,10 @@
 /**
  * This is a helper for GSM 04.11 8.2.5.2 Destination address element
  */
-void sms_encode_addr_element(struct msgb *out, const char *new_number,
+static bool sms_encode_addr_element(struct msgb *out, const char *new_number,
 			     int format, int tp_data)
 {
-	uint8_t new_addr_len;
+	int new_addr_len;
 	uint8_t new_addr[26];
 
 	/*
@@ -355,6 +355,9 @@
 	 */
 	new_addr_len = gsm48_encode_bcd_number(new_addr, ARRAY_SIZE(new_addr),
 					       1, new_number);
+	if (new_addr_len < 0)
+		return false;
+
 	new_addr[1] = format;
 	if (tp_data) {
 		uint8_t *data = msgb_put(out, new_addr_len);
@@ -363,6 +366,8 @@
 	} else {
 		msgb_lv_put(out, new_addr_len - 1, new_addr + 1);
 	}
+
+	return true;
 }
 
 static struct msgb *sms_create_new(uint8_t type, uint8_t ref,
@@ -391,7 +396,10 @@
 	msgb_v_put(out, ref);
 	msgb_lv_put(out, orig_addr_len, orig_addr_ptr);
 
-	sms_encode_addr_element(out, new_number, 0x91, 0);
+	if (!sms_encode_addr_element(out, new_number, 0x91, 0)) {
+		LOGP(DNAT, LOGL_ERROR, "Failed to encode SMS address.\n");
+		return NULL;
+	}
 
 
 	/* Patch the TPDU from here on */
@@ -411,11 +419,17 @@
 		msgb_v_put(out, data_ptr[1]);
 
 		/* encode the new number and put it */
-		if (strncmp(new_dest_nr, "00", 2) == 0)
-			sms_encode_addr_element(out, new_dest_nr + 2, 0x91, 1);
-		else
-			sms_encode_addr_element(out, new_dest_nr, 0x81, 1);
-
+		if (strncmp(new_dest_nr, "00", 2) == 0) {
+			if (!sms_encode_addr_element(out, new_dest_nr + 2, 0x91, 1)) {
+				LOGP(DNAT, LOGL_ERROR, "Failed to encode SMS address.\n");
+				return NULL;
+			}
+		} else {
+			if (!sms_encode_addr_element(out, new_dest_nr, 0x81, 1)) {
+				LOGP(DNAT, LOGL_ERROR, "Failed to encode SMS address.\n");
+				return NULL;
+			}
+		}
 		/* Copy the rest after the TP-DS */
 		data = msgb_put(out, data_len - 2 - 1 - old_dest_len);
 		memcpy(data, &data_ptr[2 + 1 + old_dest_len], data_len - 2 - 1 - old_dest_len);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I9fc16e24f7df5ebad6f4f1b389b2c5e861be95d7
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>