fixeria has uploaded this change for review. ( 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()
Cap num_digits to the size of gt->digits before decoding. Add a unit test covering both the exact-fit and oversized num_digits cases.
Change-Id: I59f601f2d8706748797c802f0f09887e4b9ba31f Fixes: OS#7046 --- M src/sua.c M tests/xua/xua_test.c M tests/xua/xua_test.ok 3 files changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/28/43228/1
diff --git a/src/sua.c b/src/sua.c index b169d50..50fe030 100644 --- a/src/sua.c +++ b/src/sua.c @@ -400,6 +400,10 @@ gt->npi = data[6]; gt->nai = data[7];
+ /* cap num_digits to what fits into gt->digits (leaving room for '\0') */ + if (num_digits > sizeof(gt->digits) - 1) + num_digits = sizeof(gt->digits) - 1; + /* parse digits */ out_digits = gt->digits; for (i = 0; i < datalen-8; i++) { diff --git a/tests/xua/xua_test.c b/tests/xua/xua_test.c index 3c5f5fd..ac044be 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)) == 0); + OSMO_ASSERT(strlen(gt.digits) == sizeof(gt.digits) - 1); + printf("OUT:%s\n", osmo_sccp_gt_dump(>)); + + data[4] = 0xff; /* num_digits: attacker-controlled, way too large */ + printf("Testing sua_parse_gt() with oversized num_digits\n"); + OSMO_ASSERT(sua_parse_gt(>, data, sizeof(data)) == 0); + 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..d4a70b1 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 oversized num_digits +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