fixeria has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/43191?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted
one.
)Change subject: gsm0480: fix out-of-bounds read in parse_process_uss_req()
......................................................................
gsm0480: fix out-of-bounds read in parse_process_uss_req()
The USSD-String octet count (uss_req_data[6]) was only checked
against the 160-byte GSM0480_USSD_OCTET_STRING_LEN destination limit,
never against the number of bytes actually remaining in the received
message. A truncated ProcessUnstructuredSS-Request (as short as 8
bytes) with a length byte up to 160 caused the subsequent memcpy()
(and, for the default alphabet, gsm_7bit_decode_n_ussd()) to read up
to ~159 bytes past the end of the message buffer.
uss_req_data/length come straight from a received L3 message via the
public gsm0480_decode_ss_request() API, so this is reachable with
network-supplied input.
Add the missing length check, mirroring the one already present in
the sibling parse_process_uss_data(), and add a regression test with
an oversized length byte.
Change-Id: I62a2ea539238d8c2dbab897ce2f80b9ab905a108
Fixes: OS#7045
---
M src/gsm/gsm0480.c
M tests/ussd/ussd_test.c
M tests/ussd/ussd_test.ok
3 files changed, 34 insertions(+), 0 deletions(-)
Approvals:
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c
index 7a7f71f..a8eac6b 100644
--- a/src/gsm/gsm0480.c
+++ b/src/gsm/gsm0480.c
@@ -704,6 +704,8 @@
dcs = uss_req_data[4];
/* Get the amount of bytes */
num_chars = uss_req_data[6];
+ if (num_chars > length - 7)
+ return 0;
/* Drop messages with incorrect length */
if (num_chars > GSM0480_USSD_OCTET_STRING_LEN) {
diff --git a/tests/ussd/ussd_test.c b/tests/ussd/ussd_test.c
index 2b8321d..b131a89 100644
--- a/tests/ussd/ussd_test.c
+++ b/tests/ussd/ussd_test.c
@@ -41,6 +41,16 @@
0x05, 0x02, 0x01, 0x24
};
+/* Same REGISTER/ProcessUssReq message as ussd_request[], except the
+ * USSD-String octet count (index 18) claims 100 bytes while the message
+ * only ever carries 6. Must be rejected, not read past the buffer end. */
+static const uint8_t ussd_process_uss_req_overflow[] = {
+ 0x0b, 0x7b, 0x1c, 0x15, 0xa1, 0x13, 0x02, 0x01,
+ 0x03, 0x02, 0x01, 0x3b, 0x30, 0x0b, 0x04, 0x01,
+ 0x0f, 0x04, 0x64, 0x2a, 0xd5, 0x4c, 0x16, 0x1b,
+ 0x01, 0x7f, 0x01, 0x00
+};
+
static const uint8_t interrogate_ss[] = {
0x0b, 0x7b, 0x1c, 0x0d, 0xa1, 0x0b, 0x02, 0x01,
0x03, 0x02, 0x01, 0x0e, 0x30, 0x03, 0x04, 0x01,
@@ -221,6 +231,23 @@
printf("\n");
}
+/* parse_process_uss_req() must reject a USSD-String octet count that
+ * exceeds the bytes actually remaining in the message, rather than only
+ * capping it against GSM0480_USSD_OCTET_STRING_LEN and reading past the
+ * end of the buffer. */
+static void test_process_uss_req_overflow(void)
+{
+ int rc;
+
+ printf("[i] Testing parse_process_uss_req() against an oversized "
+ "USSD-String length\n");
+
+ rc = parse_ussd(ussd_process_uss_req_overflow,
sizeof(ussd_process_uss_req_overflow));
+ OSMO_ASSERT(rc == 0);
+
+ printf("\n");
+}
+
int main(int argc, char **argv)
{
struct ss_request req;
@@ -237,6 +264,9 @@
/* Test gsm0480_parse_facility_ie() */
test_parse_facility_ie();
+ /* Test parse_process_uss_req() against an oversized length byte */
+ test_process_uss_req_overflow();
+
memset(&req, 0, sizeof(req));
gsm0480_decode_ss_request((struct gsm48_hdr *) ussd_request,
sizeof(ussd_request), &req);
diff --git a/tests/ussd/ussd_test.ok b/tests/ussd/ussd_test.ok
index 1137080..6e23da3 100644
--- a/tests/ussd/ussd_test.ok
+++ b/tests/ussd/ussd_test.ok
@@ -12,6 +12,8 @@
[?] Data length: expected 0x01, decoded 0x01
[?] Data: expected 32, decoded 32
+[i] Testing parse_process_uss_req() against an oversized USSD-String length
+
Tested if it still works. Text was: **321#
interrogateSS CFU text..'' code 33
Testing parsing a USSD request and truncated versions
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43191?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62a2ea539238d8c2dbab897ce2f80b9ab905a108
Gerrit-Change-Number: 43191
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
