pespin has uploaded this change for review.

View Change

smpp: Fix potential write buffer overflow on sms->user_data

The writes to sms->user_data in submit_to_sms() were not being validated
against the maximum size of the buffer, which could lead into writing
past the buffer limits.
In order to assure safe encdoing of septets into the buffer, the new
libosmocore gsm_septet_pack2() is required.

Related: OS#7059
Reported-By: Adam Bedard <adam.bedard@gmail.com>
Change-Id: Iee701a3b033d78244fc10dd8563a094b8de385c0
---
M TODO-RELEASE
M src/libsmpputil/smpp_msc.c
2 files changed, 25 insertions(+), 5 deletions(-)

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

diff --git a/TODO-RELEASE b/TODO-RELEASE
index c8858d8..84a2985 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -9,3 +9,4 @@
 #library	what			description / commit summary line
 libosmovty	>=1.12.1		working optional-multi-choice
 libosmocore >1.12.0     log_get_context(), log_{get,set}_filter(_data)()
+libosmogsm	>1.14.1 		gsm_septet_pack2()
diff --git a/src/libsmpputil/smpp_msc.c b/src/libsmpputil/smpp_msc.c
index d68dcc0..5fdba36 100644
--- a/src/libsmpputil/smpp_msc.c
+++ b/src/libsmpputil/smpp_msc.c
@@ -132,6 +132,7 @@
 	struct tlv_t *t;
 	int mode;
 	int can_store_sms = ((submit->esm_class & SMPP34_MSG_MODE_MASK) != 2); /* != forward mode */
+	int rc;
 
 	dest = subscr_by_dst(net, submit->dest_addr_npi,
 			     submit->dest_addr_ton,
@@ -247,21 +248,39 @@
 			ud_len = *sms_msg + 1;
 			if (ud_len > sms_msg_len) {
 				sms_free(sms);
-				LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n", ud_len,
-				     sms_msg_len);
+				LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n",
+				     ud_len, sms_msg_len);
+				return ESME_RINVPARLEN;
+			}
+			if (ud_len > sizeof(sms->user_data)) {
+				sms_free(sms);
+				LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n",
+				     sms_msg_len, sizeof(sms->user_data));
 				return ESME_RINVPARLEN;
 			}
 			printf("copying %u bytes user data...\n", ud_len);
-			memcpy(sms->user_data, sms_msg,
-				OSMO_MIN(ud_len, sizeof(sms->user_data)));
+			memcpy(sms->user_data, sms_msg, ud_len);
 			sms_msg += ud_len;
 			sms_msg_len -= ud_len;
 			padbits = 7 - (ud_len % 7);
 		}
-		gsm_septet_pack(sms->user_data+ud_len, sms_msg, sms_msg_len, padbits);
+		rc = gsm_septet_pack2(sms->user_data + ud_len, sizeof(sms->user_data) - ud_len,
+				      sms_msg, sms_msg_len, padbits);
+		if (rc < 0) {
+			sms_free(sms);
+			LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u + sms_msg_len=%u > %zu\n",
+			     ud_len, sms_msg_len, sizeof(sms->user_data));
+			return ESME_RINVPARLEN;
+		}
 		sms->user_data_len = (ud_len*8 + padbits)/7 + sms_msg_len;/* SEPTETS */
 		/* FIXME: sms->text */
 	} else {
+		if (sms_msg_len > sizeof(sms->user_data)) {
+			sms_free(sms);
+			LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n",
+			     sms_msg_len, sizeof(sms->user_data));
+			return ESME_RINVPARLEN;
+		}
 		memcpy(sms->user_data, sms_msg, sms_msg_len);
 		sms->user_data_len = sms_msg_len;
 	}

To view, visit change 43253. To unsubscribe, or for help writing mail filters, visit settings.