fix(core): fix nrf tlv offset calculatiion wrap
What the developer wrote
Authored by tychovrahe
fix(core): fix nrf tlv offset calculatiion wrap
[no changelog]
What changed, and why it matters
This commit fixes a bug in the Trezor hardware wallet's firmware update code for the nRF wireless chip. The code reads update image headers from an untrusted source and calculates where to look for a SHA-256 hash inside the image. Before the fix, the calculation used a signed offset type and did not check whether the claimed image size was larger than the actual buffer. A malicious or malformed update image could make the offset wrap around (become a small number due to integer overflow), bypassing later safety checks and potentially causing the device to read memory outside the intended buffer. The fix rejects images whose claimed size exceeds the buffer and uses an unsigned type for the offset calculation.
Treat this as a security-relevant hardening fix. Verify whether this code path is reachable from untrusted input (e.g., USB/Bluetooth firmware update commands or malicious update files) and backport the bounds check to other branches. Consider adding similar img_size > binary_size checks elsewhere in the nRF update code and reviewing other uses of off_t for untrusted offsets.
Security signals we found
Integer overflow/wrap in offset calculation on attacker-controlled header fields
Out-of-bounds read primitive in firmware image parsing
Missing size validation against buffer bounds
Signed offset type used for memory arithmetic
Patch adds explicit bounds check and switches to unsigned arithmetic
Evidence from the diff
In core/embed/io/nrf/stm32u5/nrf_update.c, read_image_sha256() parses an MCUboot-style image header to locate the SHA-256 TLV in the image trailer. The header fields (ih_img_size, ih_hdr_size, ih_protect_tlv_size) are attacker-controlled/untrusted. Previously, the code computed off_t off = 0 + hdr_size + img_size + tvl1_size + 4; without validating img_size against binary_size. Because off_t is signed, a sufficiently large hdr_size/img_size/tvl1_size combination could wrap to a small positive or negative value, passing the subsequent off + 8 > binary_size bounds check and causing out-of-bounds reads while scanning TLVs. The patch adds if (img_size > binary_size) return false; and changes the offset type to uint32_t. The comment notes that with this check, the maximum overshoot is bounded by 0x20002 because the other two size fields are 16-bit header entries.
Changed components
core/embed/io/nrf/stm32u5/nrf_update.cTrezor Core firmware nRF update/verification pathMCUboot image SHA-256 TLV parsingInspect captured patch +10 / −1
### core/embed/io/nrf/stm32u5/nrf_update.c
@@ -62,8 +62,17 @@ static bool read_image_sha256(const uint8_t *binary_ptr, size_t binary_size,
uint32_t hdr_size = hdr->ih_hdr_size;
uint32_t tvl1_size = hdr->ih_protect_tlv_size;
+ /* The header fields are untrusted, and a wrapped offset would pass the
+ bounds checks below as a small one. An image claiming more bytes than the
+ buffer holds is invalid anyway, and rejecting it also bounds the sum: the
+ other two fields come from 16-bit header entries, so `off` can exceed
+ `binary_size` by at most 0x20002. */
+ if (img_size > binary_size) {
+ return false;
+ }
+
/* Compute start of TLV trailer */
- off_t off = 0 + hdr_size + img_size + tvl1_size + 4;
+ uint32_t off = hdr_size + img_size + tvl1_size + 4;
/* Scan TLVs until we find the SHA-256 entry */
while (true) {Why this scored 61/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.