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