fix(core): tie ble::read()'s length to its buffer
What the developer wrote
Authored by tychovrahe
fix(core): tie ble::read()'s length to its buffer
`read()` took the buffer and the length the caller claims for it as separate
arguments, and passed the length straight to `ble_read()`. `ble_read()` writes
a full `BLE_RX_PACKET_SIZE` packet whenever the length it is given is at least
that big, so a caller passing a length larger than its buffer gets 244 bytes
written into something shorter.
The one caller, `py_iface_read()`, hands `read()` a Python-supplied buffer of
arbitrary size and passed the constant `RX_PACKET_SIZE`. It checks
`buf.len() < RX_PACKET_SIZE` first, so nothing is wrong today - but that check
is the only thing standing between a short bytearray and an overflow.
Derive the length from the buffer so the two cannot disagree. `ble_read()`
then refuses to write at all when the buffer is too small, and the check in
`py_iface_read()` is left to produce a clearer error rather than to keep us in
bounds. Saturate rather than fail the conversion: the length is only compared
against the packet size, so a buffer above 65535 bytes must keep working.
[no changelog]
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This commit fixes a potential buffer overflow in the Bluetooth Low Energy (BLE) read function of the Trezor hardware wallet firmware. Previously, the function trusted the caller's claim about how large the buffer was, which could allow more data to be written into a smaller memory area than intended. The fix makes the function use the actual buffer size instead, removing that trust relationship and preventing a possible overflow.
Treat as a security-hardening fix. Review other Rust FFI wrappers for similar buffer/length split patterns. No immediate incident response is indicated because the current caller already enforces the size constraint, but the fix removes a fragile trust boundary that could become exploitable if the caller's check is ever bypassed or changed.
Security signals we found
Potential buffer overflow due to mismatched buffer/length arguments in unsafe FFI boundary
Removal of caller-supplied length in favor of buffer-derived length
Defensive API hardening in Bluetooth transport layer
Saturating cast from usize to u16 to avoid panics on oversized buffers
Evidence from the diff
The read() function in core/embed/rust/src/trezorhal/ble/mod.rs previously accepted both a mutable buffer slice (&mut [u8]) and a separate max_len: usize argument, passing max_len directly to the unsafe FFI call ble_read(). The underlying ble_read() writes a full BLE_RX_PACKET_SIZE (244 bytes) whenever the supplied length is at least that large, so a caller passing a length larger than the buffer’s actual capacity could cause a stack/heap buffer overflow. The only caller, py_iface_read(), currently checks buf.len() < RX_PACKET_SIZE before calling read(), so the vulnerability is not directly exploitable today, but the safety of the Rust abstraction depended entirely on that external check. The patch removes the max_len parameter and derives the length from buf.len(), saturating at u16::MAX to preserve behavior for oversized buffers. This is a defensive hardening fix that eliminates a class of length mismatch bugs.
Changed components
core/embed/rust/src/trezorhal/ble/mod.rscore/embed/rust/src/trezorhal/ble/micropython.rsBLE read path between MicroPython interface and trezorhal FFIInspect captured patch +6 / −3
### core/embed/rust/src/trezorhal/ble/micropython.rs
@@ -252,7 +252,7 @@ extern "C" fn py_iface_read(n_args: usize, args: *const Obj) -> Obj {
if buf.len() < RX_PACKET_SIZE {
return Err(Error::ValueError(c"Buffer too small"));
}
- let read_len = read(buf, RX_PACKET_SIZE)?;
+ let read_len = read(buf)?;
if read_len != RX_PACKET_SIZE {
return Err(Error::ValueError(c"Unexpected read length"));
}
### core/embed/rust/src/trezorhal/ble/mod.rs
@@ -208,8 +208,11 @@ pub fn write(bytes: &[u8]) -> Result<(), Error> {
}
}
-pub fn read(buf: &mut [u8], max_len: usize) -> Result<usize, Error> {
- let len: u16 = max_len.try_into()?;
+pub fn read(buf: &mut [u8]) -> Result<usize, Error> {
+ // Derived from `buf` so the two cannot disagree. `ble_read` writes a
+ // fixed-size packet and uses this only to check the buffer is big enough,
+ // so saturating a larger buffer to u16::MAX changes nothing.
+ let len = buf.len().min(u16::MAX as usize) as u16;
let read_len = unsafe { super::ffi::ble_read(buf.as_mut_ptr(), len) };
Ok(read_len.try_into()?)
}Why this scored 58/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.