fix(nordic): bound the length of SPI frames from the host MCU
What the developer wrote
Authored by tychovrahe
fix(nordic): bound the length of SPI frames from the host MCU
process_rx_msg() copied `len` bytes into trz_packet_t.data[254] without
checking it. `msg_len` is a uint8_t, so the destination could be overflowed
by one byte - landing in the packet's own `len` field - and the source read
could run two bytes past the 254-byte SPI receive buffer, whose data field
starts at offset 2 and is 251 bytes long.
Bound both ends: reject frames whose msg_len exceeds MAX_SPI_DATA_SIZE while
the frame is being validated, and keep a check against PACKET_DATA_SIZE in
process_rx_msg(), which owns the destination.
[no changelog]
What changed, and why it matters
This commit fixes a small but real memory overflow bug in the code that handles Bluetooth-related SPI messages between the host microcontroller and the Nordic chip in a Trezor device. A malicious or malformed message could ask the firmware to copy slightly more data than the destination buffer can hold, potentially corrupting an adjacent length field and reading past the end of the receive buffer. The patch now rejects oversized frames at two different points in the code.
Treat this as a security fix and include it in release notes or a changelog entry. Review whether the same pattern exists in other transport paths. Consider fuzzing the SPI/ BLE message parsers. No independent CVE is required unless the vendor determines exploitability warrants one.
Security signals we found
Buffer overflow / out-of-bounds write in packet handling
Out-of-bounds read from SPI receive buffer
Missing length validation on attacker-influenced message field
Defense-in-depth: bounds check added at both validation and consumption layers
Evidence from the diff
process_rx_msg() in trz_comm.c copies len bytes into trz_packet_t.data[254] without first validating len. Because msg_len is a uint8_t, the destination can be overflowed by up to one byte into the packet’s own len field, and the source read can run up to two bytes past the 251-byte SPI data region (the receive buffer’s data field starts at offset 2). The patch adds a length check in spi.c during frame validation (msg_len <= MAX_SPI_DATA_SIZE) and a redundant guard in process_rx_msg() (len > PACKET_DATA_SIZE).
Changed components
nordic/trezor/trezor-ble/src/trz_comm/spi.cnordic/trezor/trezor-ble/src/trz_comm/trz_comm.cTrezor BLE/nRF5x SPI transport layerInspect captured patch +9 / −3
### nordic/trezor/trezor-ble/src/trz_comm/spi.c
@@ -198,7 +198,8 @@ void spi_thread(void) {
uint8_t crc = crc8(rx_data, PACKET_DATA_SIZE - 1, 0x07, 0, false);
- if (crc == rx_msg->crc && (rx_msg->service_id & 0xF0) == 0xA0) {
+ if (crc == rx_msg->crc && (rx_msg->service_id & 0xF0) == 0xA0 &&
+ rx_msg->msg_len <= MAX_SPI_DATA_SIZE) {
process_rx_msg(rx_msg->service_id & 0xF, rx_msg->data, rx_msg->msg_len);
} else {
if (rx_msg->service_id != 0) {
### nordic/trezor/trezor-ble/src/trz_comm/trz_comm.c
@@ -61,10 +61,15 @@ bool trz_comm_send_msg(nrf_service_id_t service, const uint8_t *data,
}
void process_rx_msg(uint8_t service_id, uint8_t *data, uint32_t len) {
- trz_packet_t *buf = k_malloc(sizeof(*buf));
-
atomic_set(&g_suspended_flag, 0);
+ if (len > PACKET_DATA_SIZE) {
+ LOG_WRN("Received message too long (%u bytes), dropping", len);
+ return;
+ }
+
+ trz_packet_t *buf = k_malloc(sizeof(*buf));
+
if (!buf) {
LOG_WRN("Not able to allocate receive buffer");
return;Why this scored 49/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.