fixeria has submitted this change. ( 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, 35 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
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..f020aee 100644
--- a/tests/gsm29205/gsm29205_test.c
+++ b/tests/gsm29205/gsm29205_test.c
@@ -95,6 +95,34 @@
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("\tosmo_dec_gcr(len=%u) -> %s\n",
+ len, rc == len ? "OK" : "FAIL");
+ }
+}
+
int main(int argc, char **argv)
{
osmo_init_logging2(talloc_named_const(NULL, 0, "gsm29205 test"), NULL);
@@ -102,6 +130,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..cc9b7d2 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...
+ osmo_dec_gcr(len=13) -> FAIL
+ osmo_dec_gcr(len=14) -> FAIL
+ osmo_dec_gcr(len=15) -> OK
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: merged
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I6855d6c810c2b4274ccfd7bc861405f9b4e09343
Gerrit-Change-Number: 43190
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: Hoernchen, dexter, laforge.
Hello Jenkins Builder, dexter, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/43172?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Verified-1 by Jenkins Builder
Change subject: GP: LOAD/STORE DATA chunk size from SCP overhead
......................................................................
GP: LOAD/STORE DATA chunk size from SCP overhead
SCP.overhead was so far set at construction time (SCP02: 8, SCP03:
s_mode), so the C-MAC length only.
Unfortunately sec lvl >= 3 pads the data field to the cipher block size
before encryption, so the real worst-case overhead is larger,
scc.max_cmd_len (255 - overhead) was too big, and ADF_SD.load()
used a hardcoded chunk_len=240.
Real world issue with a 286 byte CAP + SCP02 + sec lvl 3:
- 240-byte LOAD block is padded to 248,
- encrypted
- gets 8 byte C-MAC appended
-> Lc = 256
That dies with a weird "ValueError: bytes must be in range(0, 256)".
The only "fix" for that was to downgrade the seclevel.
STORE DATA has the same overflow with large max_cmd_len
(247 + padding + MAC = 256 as well).
Therefore the overhead must be properly calculated from the sec level.
While at it adjust the error in case I missed something to get a more
useful ValueError.
Change-Id: Ic208f3959a38896f64fb6ccefb24cc360a3ac3a2
---
M pySim/global_platform/__init__.py
M pySim/global_platform/scp.py
M tests/unittests/test_globalplatform.py
3 files changed, 290 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/72/43172/6
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/43172?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ic208f3959a38896f64fb6ccefb24cc360a3ac3a2
Gerrit-Change-Number: 43172
Gerrit-PatchSet: 6
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Attention is currently required from: dexter, fixeria, laforge, lynxis lazus.
Hello Jenkins Builder, dexter, fixeria, laforge, lynxis lazus,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/pysim/+/43136?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Code-Review+1 by fixeria, Code-Review+1 by laforge, Verified+1 by Jenkins Builder
Change subject: GP: mixed PSK TLS PUT KEY (Amendment B Table 3-13)
......................................................................
GP: mixed PSK TLS PUT KEY (Amendment B Table 3-13)
AES PSK + DES DEK for scp81, tested with sysmoEUICC1 C2T
Change-Id: I480a9d049a052aa5ae54fe6e2771dba44e89434d
---
M pySim/global_platform/__init__.py
M tests/unittests/test_globalplatform.py
2 files changed, 226 insertions(+), 19 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/36/43136/5
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/43136?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I480a9d049a052aa5ae54fe6e2771dba44e89434d
Gerrit-Change-Number: 43136
Gerrit-PatchSet: 5
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Attention is currently required from: dexter, fixeria, lynxis lazus.
Hoernchen has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/43136?usp=email )
Change subject: GP: mixed PSK TLS PUT KEY (Amendment B Table 3-13)
......................................................................
Patch Set 3:
(1 comment)
File pySim/global_platform/__init__.py:
https://gerrit.osmocom.org/c/pysim/+/43136/comment/289459c0_2ca538ce?usp=em… :
PS3, Line 664: build_construct
> I am wondering why do you need `build_construct()` and why can't you just do `cls.KeyDataBasic. […]
Well this is just like the original code which did b2h in the arg parsing and then build_construct() in put key and I tried to stick to things that are known to work..
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/43136?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I480a9d049a052aa5ae54fe6e2771dba44e89434d
Gerrit-Change-Number: 43136
Gerrit-PatchSet: 3
Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 11 Aug 2026 15:09:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Attention is currently required from: Hoernchen, dexter, osmith.
pespin has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43203?usp=email )
Change subject: smdpp: update SGP.26 DPtls certificate
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I do wonder, can't we generate this certificate as part of the test run, in order to avoid submitting a commit every X months containing binary blobs?
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/43203?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: I09c70797e2ba897a41d46c378dfd0ca12b645a79
Gerrit-Change-Number: 43203
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 11 Aug 2026 15:07:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: dexter, jolly.
Hello Jenkins Builder, dexter,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/onomondo-ipa/+/43202?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+2 by dexter
The change is no longer submittable: Code-Review is unsatisfied now.
Change subject: Introduce 'operation' option to main file
......................................................................
Introduce 'operation' option to main file
Instead of having several command line flags to trigger various
operations, a single command line option '-o <operation>' is used.
Because an operation runs exclusively, it makes no sense to allow
multiple command line flags at the same time.
New operations are added in later patches, so they can use the '-o'
option as well and do not need to introduce new command line flags.
Revert from getopt_long() back to getopt(), because long options are not
required anymore.
Related: SYS#8101
Change-Id: I6b960f840820990de40fb07332669ddbfcdf1e7d
---
M src/ipa/main.c
1 file changed, 27 insertions(+), 21 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/onomondo-ipa refs/changes/02/43202/2
--
To view, visit https://gerrit.osmocom.org/c/onomondo-ipa/+/43202?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: onomondo-ipa
Gerrit-Branch: master
Gerrit-Change-Id: I6b960f840820990de40fb07332669ddbfcdf1e7d
Gerrit-Change-Number: 43202
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>