fixeria submitted this change.

View Change

sua: fix buffer overflow in sua_parse_gt()

RFC 3868 does not impose a limit on the Number of Digits, so ideally
we should be able to parse up to 255 digits.  However, our
gt->digits[] can only fit up to 31 digits + a terminating NUL.

Cap num_digits to the size of gt->digits before decoding and parse
as much as we can, returning -ENOSPC if not all digits fit.  Add a
unit test covering both the exact-fit and oversized num_digits cases.

Change-Id: I59f601f2d8706748797c802f0f09887e4b9ba31f
Reported-By: 3ntr0py1337
Fixes: OS#7046
---
M src/sua.c
M tests/xua/xua_test.c
M tests/xua/xua_test.ok
3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/src/sua.c b/src/sua.c
index f5dcd66..35c4a8e 100644
--- a/src/sua.c
+++ b/src/sua.c
@@ -383,12 +383,14 @@
  *  \param[out] gt User-allocated structure for decoded output
  *  \param[in] data binary-encoded data
  *  \param[in] datalen length of \ref data in octets
+ *  \returns 0 on success; negative on error
  */
 int sua_parse_gt(struct osmo_sccp_gt *gt, const uint8_t *data, unsigned int datalen)
 {
 	uint8_t num_digits;
 	char *out_digits;
 	unsigned int i;
+	int rc = 0;
 
 	/* 8 byte header at minimum, plus digits */
 	if (datalen < 8)
@@ -401,6 +403,15 @@
 	gt->npi = data[6];
 	gt->nai = data[7];
 
+	/* XXX: RFC 3868 does not impose a limit on the Number of Digits, so
+	 * ideally we should be able to parse up to 255 digits.  However, our
+	 * gt->digits[] can only fit up to 31 digits + a terminating NUL. */
+	if (num_digits > sizeof(gt->digits) - 1) {
+		/* Parse as much as we can; return -ENOSPC */
+		num_digits = sizeof(gt->digits) - 1;
+		rc = -ENOSPC;
+	}
+
 	/* parse digits */
 	out_digits = gt->digits;
 	for (i = 0; i < datalen-8; i++) {
@@ -414,7 +425,7 @@
 	}
 	*out_digits++ = '\0';
 
-	return 0;
+	return rc;
 }
 
 /*! \brief parse SCCP address from given xUA message part
diff --git a/tests/xua/xua_test.c b/tests/xua/xua_test.c
index 3c5f5fd..dcd2089 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)) == -ENOSPC);
+	OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1);
+	printf("OUT:%s\n", osmo_sccp_gt_dump(&gt));
+
+	data[4] = 0xff;			/* num_digits: way too large */
+	printf("Testing sua_parse_gt() with num_digits=%u\n", data[4]);
+	OSMO_ASSERT(sua_parse_gt(&gt, data, sizeof(data)) == -ENOSPC);
+	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..f6f30d6 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 num_digits=255
+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.