[S] Change in libosmo-sigtran[master]: sua: sua_parse_gt(): reject GT with num_digits exceeding blob length

[fixeria]

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(&gt));
 }

+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(&gt, data, sizeof(data)) == -EINVAL);
+       OSMO_ASSERT(strlen(gt.digits) == (sizeof(data) - 8) * 2);
+       printf("OUT:%s\n", osmo_sccp_gt_dump(&gt));
+}
+
 /* 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

--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/43349?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I918278b6471bc78511bf97a292204e6719d4031a
Gerrit-Change-Number: 43349
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanits...@sysmocom.de>