fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43349?usp=email )
Change subject: sua: sua_parse_gt(): reject GT with num_digits exceeding blob length ......................................................................
sua: sua_parse_gt(): reject GT with num_digits exceeding blob length
The digit-parsing loop only clamped num_digits against the capacity of gt->digits[], but never validated it against the actual length of the given input. A blob claiming more digits than it carries was silently truncated to whatever data happened to be available, without any error indication to the caller.
Add an explicit check of num_digits against datalen before the parsing loop, alongside the existing gt->digits[] capacity check. This keeps the loop body free of per-iteration bounds checks, and reports -EINVAL to the caller for such malformed input.
Change-Id: I918278b6471bc78511bf97a292204e6719d4031a Related: OS#7046 --- M src/sua.c M tests/xua/xua_test.c M tests/xua/xua_test.ok 3 files changed, 34 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/49/43349/1
diff --git a/src/sua.c b/src/sua.c index 7c6d57f..be18d5c 100644 --- a/src/sua.c +++ b/src/sua.c @@ -412,16 +412,20 @@ rc = -ENOSPC; }
- /* parse digits: bounded by num_digits (already clamped above), so this - * can never write more than sizeof(gt->digits)-1 nibbles into gt->digits[] */ + /* claimed num_digits must fit into the remaining input octets */ + if (num_digits > (datalen - 8) * 2) { + /* Parse as much as we can; return -EINVAL */ + num_digits = (datalen - 8) * 2; + rc = -EINVAL; + } + + /* parse digits: bounded by num_digits + * (already clamped above to fit both gt->digits[] and the input data) */ out_digits = gt->digits; for (i = 0; i < num_digits; i++) { unsigned int byte_off = 8 + i / 2; - uint8_t byte; + uint8_t byte = data[byte_off];
- if (byte_off >= datalen) - break; - byte = data[byte_off]; if (i & 1) *out_digits++ = osmo_bcd2char(byte >> 4); else diff --git a/tests/xua/xua_test.c b/tests/xua/xua_test.c index dcd2089..76fe9db 100644 --- a/tests/xua/xua_test.c +++ b/tests/xua/xua_test.c @@ -397,6 +397,27 @@ printf("OUT:%s\n", osmo_sccp_gt_dump(>)); }
+static void test_sua_parse_gt_truncated(void) +{ + /* 8-byte header + only 2 digit octets (4 digits worth of data) */ + uint8_t data[8 + 2]; + 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 */ + + /* num_digits fits into gt->digits[], but claims more digits than the + * given blob actually carries */ + data[4] = 10; + printf("Testing sua_parse_gt() with num_digits=%u, datalen=%zu\n", data[4], sizeof(data)); + OSMO_ASSERT(sua_parse_gt(>, data, sizeof(data)) == -EINVAL); + OSMO_ASSERT(strlen(gt.digits) == (sizeof(data) - 8) * 2); + printf("OUT:%s\n", osmo_sccp_gt_dump(>)); +} + /* SCCP Message Transcoding */
struct sccp2sua_testcase { @@ -705,6 +726,7 @@ test_sccp_addr_parser(); test_helpers(); test_sua_parse_gt_overflow(); + test_sua_parse_gt_truncated(); test_sccp2sua(); test_rkm(); test_sccp_addr_encdec(); diff --git a/tests/xua/xua_test.ok b/tests/xua/xua_test.ok index f6f30d6..b3bc8e3 100644 --- a/tests/xua/xua_test.ok +++ b/tests/xua/xua_test.ok @@ -20,6 +20,8 @@ OUT:DIG=1111111111111111111111111111111 Testing sua_parse_gt() with num_digits=255 OUT:DIG=1111111111111111111111111111111 +Testing sua_parse_gt() with num_digits=10, datalen=10 +OUT:DIG=1111
=> BSSMAP-RESET SCCP Input: [L2]> 09 00 03 05 07 02 42 fe 02 42 fe 06 00 04 30 04 01 20