fixeria has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43228?usp=email )
Change subject: sua: fix buffer overflow in sua_parse_gt()
......................................................................
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(-)
Approvals:
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
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(>, data, sizeof(data)) == -ENOSPC);
+ OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1);
+ printf("OUT:%s\n", osmo_sccp_gt_dump(>));
+
+ 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(>, data, sizeof(data)) == -ENOSPC);
+ OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1);
+ printf("OUT:%s\n", osmo_sccp_gt_dump(>));
+}
+
/* 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 https://gerrit.osmocom.org/c/libosmo-sigtran/+/43228?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I59f601f2d8706748797c802f0f09887e4b9ba31f
Gerrit-Change-Number: 43228
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
