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 <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Attention is currently required from: laforge, lynxis lazus, pespin.
dexter has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/libosmocore/+/43178?usp=email )
Change subject: RFC: sim/class_tables: add a size attribute
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> Then maybe leave the public struct and API there as deprecated, and create a new API with "2" appene […]
simtrace2/host/lib/apdu_dispatch.c seems to be an API user of this.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43178?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Iee50063399a0c3b29594e737f44aaa125fd06a2e
Gerrit-Change-Number: 43178
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Mon, 10 Aug 2026 09:43:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: dexter.
laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43182?usp=email )
Change subject: eim_Tests: Add testcase to test ESipa/ES9+: CancelSession
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43182?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Iadc9010fcf08de98f7d8c2a5c68bb5ff71bfb1f6
Gerrit-Change-Number: 43182
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 10 Aug 2026 09:42:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43179?usp=email )
Change subject: SGP32Definitions: remove wrong comment about workaround
......................................................................
SGP32Definitions: remove wrong comment about workaround
This patch removes a misplaced workaround comment. The ASN.1 structures
in questions are equal to the original spec and were not modified.
Related: SYS#8100
Change-Id: Id54ba6a1541c8d667e0381239fa850175609ab85
---
M library/euicc/SGP32Definitions.asn
1 file changed, 0 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/library/euicc/SGP32Definitions.asn b/library/euicc/SGP32Definitions.asn
index 93b31b2..dcfe1c5 100644
--- a/library/euicc/SGP32Definitions.asn
+++ b/library/euicc/SGP32Definitions.asn
@@ -608,11 +608,9 @@
parametersNotAvailable(1),
undefinedError(127)
}
--- workaround for erlang asn1ct: remove already specified context-specific tag
SGP32-SetDefaultDpAddressRequest ::= [101] SEQUENCE { -- Tag 'BF65'
defaultDpAddress UTF8String -- Default SM-DP+ address as an FQDN
}
--- workaround for erlang asn1ct: remove already specified context-specific tag
SGP32-SetDefaultDpAddressResponse ::= [101] SEQUENCE { -- Tag 'BF65'
setDefaultDpAddressResult INTEGER { ok (0), undefinedError (127)}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43179?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Id54ba6a1541c8d667e0381239fa850175609ab85
Gerrit-Change-Number: 43179
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>