fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43191?usp=email )
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(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/91/43191/1
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: newchange
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I62a2ea539238d8c2dbab897ce2f80b9ab905a108
Gerrit-Change-Number: 43191
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43190?usp=email )
Change subject: gsm29205: fix out-of-bounds read in osmo_dec_gcr()
......................................................................
gsm29205: fix out-of-bounds read in osmo_dec_gcr()
The length check at the top of the function only verified that the
input buffer was at least 13 bytes, which is the minimum needed for
.net_len == 3. For .net_len == 4 or 5 (also valid per the length
check further down), the actual minimum required length is
10 + net_len, i.e. 14 or 15 bytes. With a shorter buffer, the
subsequent osmo_load16be(), elem[] access and memcpy() read
past the end of the caller-supplied buffer.
elem/len are taken directly from a received BSSMAP Global Call
Reference IE (gsm0808_utils.c), so this is reachable with
network-supplied input.
Add a length check depending on the actual .net_len, and a test.
Change-Id: I6855d6c810c2b4274ccfd7bc861405f9b4e09343
Fixes: OS#7044
---
M src/gsm/gsm29205.c
M tests/gsm29205/gsm29205_test.c
M tests/gsm29205/gsm29205_test.ok
3 files changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/90/43190/1
diff --git a/src/gsm/gsm29205.c b/src/gsm/gsm29205.c
index 8fed020..8fe91f7 100644
--- a/src/gsm/gsm29205.c
+++ b/src/gsm/gsm29205.c
@@ -73,6 +73,8 @@
gcr->net_len = elem[0];
if (gcr->net_len < 3 || gcr->net_len > 5)
return -EINVAL;
+ if (len < 10 + gcr->net_len)
+ return -EBADMSG;
memcpy(gcr->net, elem + parsed, gcr->net_len);
/* +1 for ignored Node ID length field */
diff --git a/tests/gsm29205/gsm29205_test.c b/tests/gsm29205/gsm29205_test.c
index 6598f89..6bcf602 100644
--- a/tests/gsm29205/gsm29205_test.c
+++ b/tests/gsm29205/gsm29205_test.c
@@ -95,6 +95,33 @@
msgb_free(msg);
}
+/* osmo_dec_gcr() must reject buffers that are too short for the announced
+ * .net_len (3..5), not just shorter than the 13-byte minimum for
+ * .net_len == 3. Otherwise it reads past the end of 'elem'. */
+static void test_gcr_dec_short_buf(void)
+{
+ static const uint8_t res[] = {
+ 0x05, /* .net_len */
+ 0x51, 0x52, 0x53, 0x54, 0x55, /* .net */
+ 0x02, /* .node length */
+ 0xde, 0xad, /* .node */
+ 0x05, /* length of Call. Ref. */
+ 0x41, 0x42, 0x43, 0x44, 0x45 /* .cr - Call. Ref. */
+ };
+ struct osmo_gcr_parsed p;
+ uint8_t len;
+ int rc;
+
+ printf("Testing Global Call Reference decoder against short buffers...\n");
+
+ /* net_len == 5 requires 15 bytes, feed it 13 and 14
+ * the full buffer must still decode successfully */
+ for (len = 13; len <= ARRAY_SIZE(res); len++) {
+ rc = osmo_dec_gcr(&p, res, len);
+ printf("\tlen=%u: rc=%d\n", len, rc);
+ }
+}
+
int main(int argc, char **argv)
{
osmo_init_logging2(talloc_named_const(NULL, 0, "gsm29205 test"), NULL);
@@ -102,6 +129,7 @@
printf("Testing 3GPP TS 29.205 routines...\n");
test_gcr();
+ test_gcr_dec_short_buf();
printf("Done.\n");
diff --git a/tests/gsm29205/gsm29205_test.ok b/tests/gsm29205/gsm29205_test.ok
index bddd88a..d83db2e 100644
--- a/tests/gsm29205/gsm29205_test.ok
+++ b/tests/gsm29205/gsm29205_test.ok
@@ -2,4 +2,8 @@
Testing Global Call Reference encoder...
13 bytes added: OK
decoded 13 bytes: OK
+Testing Global Call Reference decoder against short buffers...
+ len=13: rc=-74
+ len=14: rc=-74
+ len=15: rc=15
Done.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43190?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6855d6c810c2b4274ccfd7bc861405f9b4e09343
Gerrit-Change-Number: 43190
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: daniel, laforge, pespin.
lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007?usp=email )
Change subject: TCAP: Refactoring unroutable messages into own function
......................................................................
Patch Set 2:
(4 comments)
File src/tcap_as_loadshare.c:
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007/comment/87db6544_c09cc… :
PS2, Line 458: the message will be paths.
> what do you mean by "the message will be paths"? I don't understand what this is supposed to say.
Done
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007/comment/445464c7_6fc8e… :
PS2, Line 466: static int asp_loadshare_tcap_unroutable(struct osmo_ss7_asp **rasp,
> asp_loadshare_tcap_unroutable_fallback() may be more descriptive. […]
fallback will be used by the DPC routing. I've renamed it to asp_loadshare_tcap_handle_unroutable
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007/comment/c03bd5cf_e5413… :
PS2, Line 608: rate_ctr_inc2(as->ctrg, SS7_AS_CTR_TCAP_ASP_FALLBACK);
> AFAIU this patch could be reduce to only adding the following line here: […]
no, because you also have to set the rc code. In general this function needs to be refactored. it is far to long and complex.
I've refactored some of the parts. But still this function would need a refactoring
https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007/comment/9aa931d1_c8ed9… :
PS2, Line 636: rate_ctr_inc2(as->ctrg, SS7_AS_CTR_TCAP_ASP_FALLBACK);
> AFAIU this patch could be reduce to only adding the following line here: […]
see above.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/43007?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: Ib4d114273423730418458767d17b11de9bd815d4
Gerrit-Change-Number: 43007
Gerrit-PatchSet: 2
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Sat, 08 Aug 2026 12:33:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: theenbyperor.
lynxis lazus has posted comments on this change by theenbyperor. ( https://gerrit.osmocom.org/c/erlang/osmo_dia2gsup/+/43092?usp=email )
Change subject: Set Item-Number in UTRAN and GERAN vectors.
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo_dia2gsup/+/43092?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo_dia2gsup
Gerrit-Branch: master
Gerrit-Change-Id: I75da04b806f23452c9b4cfaad0446dceb32a0369
Gerrit-Change-Number: 43092
Gerrit-PatchSet: 2
Gerrit-Owner: theenbyperor <q(a)magicalcodewit.ch>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Attention: theenbyperor <q(a)magicalcodewit.ch>
Gerrit-Comment-Date: Sat, 08 Aug 2026 10:46:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
n0k0 has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-bsc/+/43188?usp=email )
Change subject: ipaccess-proxy: reject short IPA header
......................................................................
ipaccess-proxy: reject short IPA header
recv() of the 3-byte IPA header can return fewer than 3 bytes on a
stream socket. ipaccess_proxy_read_msg() only handles ret < 0 and
ret == 0, so a short read falls through: msgb_put() advances msg->tail
by 'ret' while msg->l2h is set to msg->data + sizeof(*hh). The frame
length is then validated against msgb_tailroom() (measured from
msg->tail), so the subsequent body recv() into msg->l2h can write up to
sizeof(*hh) - ret bytes past the msgb.
Reject a short header, the same way handle_udp_read() already does.
Change-Id: I3043cccd110db19984f47916c269a4c59f82b08b
---
M src/ipaccess/ipaccess-proxy.c
1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/88/43188/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/43188?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3043cccd110db19984f47916c269a4c59f82b08b
Gerrit-Change-Number: 43188
Gerrit-PatchSet: 2
Gerrit-Owner: n0k0 <osmocom(a)hacky.software>
n0k0 has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/43188?usp=email )
Change subject: ipaccess-proxy: reject short IPA header in ipaccess_proxy_read_msg
......................................................................
ipaccess-proxy: reject short IPA header in ipaccess_proxy_read_msg
recv() of the 3-byte IPA header can return fewer than 3 bytes on a
stream socket. ipaccess_proxy_read_msg() only handles ret < 0 and
ret == 0, so a short read falls through: msgb_put() advances msg->tail
by 'ret' while msg->l2h is set to msg->data + sizeof(*hh). The frame
length is then validated against msgb_tailroom() (measured from
msg->tail), so the subsequent body recv() into msg->l2h can write up to
sizeof(*hh) - ret bytes past the msgb.
Reject a short header, the same way handle_udp_read() already does.
Change-Id: I3043cccd110db19984f47916c269a4c59f82b08b
---
M src/ipaccess/ipaccess-proxy.c
1 file changed, 10 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/88/43188/1
diff --git a/src/ipaccess/ipaccess-proxy.c b/src/ipaccess/ipaccess-proxy.c
index 71190f6..b1c500c 100644
--- a/src/ipaccess/ipaccess-proxy.c
+++ b/src/ipaccess/ipaccess-proxy.c
@@ -576,6 +576,16 @@
msgb_free(msg);
*error = ret;
return NULL;
+ } else if (ret < sizeof(*hh)) {
+ /* A short (split) IPA header would leave msg->l2h (msg->data +
+ * sizeof(*hh)) ahead of msg->tail, while the frame length below is
+ * validated against msgb_tailroom() measured from msg->tail; the
+ * body recv() could then write up to sizeof(*hh) - ret bytes past
+ * the buffer. Reject it, like the other IPA read paths do. */
+ LOGP(DLINP, LOGL_ERROR, "short read of IPA header (%d)\n", ret);
+ msgb_free(msg);
+ *error = -EIO;
+ return NULL;
}
msgb_put(msg, ret);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/43188?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I3043cccd110db19984f47916c269a4c59f82b08b
Gerrit-Change-Number: 43188
Gerrit-PatchSet: 1
Gerrit-Owner: n0k0 <osmocom(a)hacky.software>
Attention is currently required from: fixeria, n0k0.
Hello Jenkins Builder, fixeria, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/42886?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: ipaccess-proxy: reject oversized IPA frame length
......................................................................
ipaccess-proxy: reject oversized IPA frame length
ipaccess_proxy_read_msg() reads the 16-bit IPA frame length from the
wire header and passes it straight as the recv() count into a msgb that
was allocated with a fixed PROXY_ALLOC_SIZE (1200) bytes, without ever
checking it against the buffer tailroom. A peer that advertises a body
length larger than the remaining buffer space makes recv() write past
the end of the heap allocation (heap buffer overflow).
Reject frames whose advertised length exceeds the msgb tailroom, the
same way the other IPA read paths bound the read to msgb_tailroom().
This issue has been assigned the CVE candidate identifier
CAN-2026-2051036.
Change-Id: I05137e114eaa99ff0e85eecccf7645c90945214f
---
M src/ipaccess/ipaccess-proxy.c
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/86/42886/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/42886?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I05137e114eaa99ff0e85eecccf7645c90945214f
Gerrit-Change-Number: 42886
Gerrit-PatchSet: 5
Gerrit-Owner: n0k0 <osmocom(a)hacky.software>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: n0k0 <osmocom(a)hacky.software>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>