fixeria has uploaded this change for review.

View Change

sua: fix buffer overflow in sua_parse_gt()

Cap num_digits to the size of gt->digits before decoding.  Add a
unit test covering both the exact-fit and oversized num_digits cases.

Change-Id: I59f601f2d8706748797c802f0f09887e4b9ba31f
Fixes: OS#7046
---
M src/sua.c
M tests/xua/xua_test.c
M tests/xua/xua_test.ok
3 files changed, 34 insertions(+), 0 deletions(-)

git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/28/43228/1

diff --git a/src/sua.c b/src/sua.c
index b169d50..50fe030 100644
--- a/src/sua.c
+++ b/src/sua.c
@@ -400,6 +400,10 @@
 	gt->npi = data[6];
 	gt->nai = data[7];
 
+	/* cap num_digits to what fits into gt->digits (leaving room for '\0') */
+	if (num_digits > sizeof(gt->digits) - 1)
+		num_digits = sizeof(gt->digits) - 1;
+
 	/* parse digits */
 	out_digits = gt->digits;
 	for (i = 0; i < datalen-8; i++) {
diff --git a/tests/xua/xua_test.c b/tests/xua/xua_test.c
index 3c5f5fd..ac044be 100644
--- a/tests/xua/xua_test.c
+++ b/tests/xua/xua_test.c
@@ -372,6 +372,31 @@
 	msgb_free(msg);
 }
 
+static void test_sua_parse_gt_overflow(void)
+{
+	/* 8-byte header + way more digit octets than fit into gt->digits[32] */
+	uint8_t data[8 + 64];
+	struct osmo_sccp_gt gt = {};
+
+	memset(data, 0x11, sizeof(data));
+	data[3] = 0x42;		/* gti */
+	data[5] = 0x00;		/* tt */
+	data[6] = 0x01;		/* npi */
+	data[7] = 0x04;		/* nai */
+
+	data[4] = sizeof(gt.digits);	/* num_digits: not enough room for '\0' */
+	printf("Testing sua_parse_gt() with num_digits=%u\n", data[4]);
+	OSMO_ASSERT(sua_parse_gt(&gt, data, sizeof(data)) == 0);
+	OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1);
+	printf("OUT:%s\n", osmo_sccp_gt_dump(&gt));
+
+	data[4] = 0xff;			/* num_digits: attacker-controlled, way too large */
+	printf("Testing sua_parse_gt() with oversized num_digits\n");
+	OSMO_ASSERT(sua_parse_gt(&gt, data, sizeof(data)) == 0);
+	OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1);
+	printf("OUT:%s\n", osmo_sccp_gt_dump(&gt));
+}
+
 /* SCCP Message Transcoding */
 
 struct sccp2sua_testcase {
@@ -679,6 +704,7 @@
 	test_isup_parse();
 	test_sccp_addr_parser();
 	test_helpers();
+	test_sua_parse_gt_overflow();
 	test_sccp2sua();
 	test_rkm();
 	test_sccp_addr_encdec();
diff --git a/tests/xua/xua_test.ok b/tests/xua/xua_test.ok
index 02e5f49..d4a70b1 100644
--- a/tests/xua/xua_test.ok
+++ b/tests/xua/xua_test.ok
@@ -16,6 +16,10 @@
     0400000001000000040000003931393936393637393338390000000000000000000000000000000000000000
 OUT:TT=0,NPL=1,NAI=4,DIG=919969679389
     0400000001000000040000003931393936393637393338390000000000000000000000000000000000000000
+Testing sua_parse_gt() with num_digits=32
+OUT:DIG=1111111111111111111111111111111
+Testing sua_parse_gt() with oversized num_digits
+OUT:DIG=1111111111111111111111111111111
 
 => BSSMAP-RESET
 SCCP Input: [L2]> 09 00 03 05 07 02 42 fe 02 42 fe 06 00 04 30 04 01 20 

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