What the developer wrote
Authored by Martin Pastyřík
fix(core): typo in tropic.c
[no changelog]
What changed, and why it matters
A single-word typo in a Trezor hardware-wallet security file meant a safety check did nothing. The code said 'false;' instead of 'return false;', so when a buffer-limit check failed, the function kept running instead of stopping. This could let an oversized copy proceed past an intended guard, potentially corrupting memory or leaking adjacent data. The fix simply adds the missing 'return'.
Review all callers of tropic_data_multi_read() to confirm whether max_data_length and out_length are attacker-influenced or derived from untrusted storage; if so, assess for out-of-bounds write or information disclosure. Backport the one-line fix to all maintained firmware branches and add regression tests covering the boundary condition.
Security signals we found
Missing return statement neutralizes a bounds check
Subsequent memcpy may write beyond intended output buffer limits
Located in secure-element/tropic driver code (core/embed/sec/tropic/tropic.c)
No changelog entry provided, reducing immediate transparency
Evidence from the diff
In core/embed/sec/tropic/tropic.c, tropic_data_multi_read() contains a bounds check: if (out_pos + copy_length > max_data_length) { false; }. The standalone ‘false;’ expression is a no-op; the intended behavior was ‘return false;’. As a result, the guard fails to abort execution, and the subsequent memcpy(&data[out_pos], &slot_buffer[prefix_length], copy_length) and out_pos += copy_length execute even when the bounds check indicates they should not. This is a classic missing-return bug that neutralizes a length validation. The patch changes ‘false;’ to ‘return false;’.
Changed components
core/embed/sec/tropic/tropic.ctropic_data_multi_read() functionTrezor Core secure-element/tropic integrationInspect captured patch +1 / −1
diff --git a/core/embed/sec/tropic/tropic.c b/core/embed/sec/tropic/tropic.c
index ed14c483..b87e420b 100644
--- a/core/embed/sec/tropic/tropic.c
+++ b/core/embed/sec/tropic/tropic.c
@@ -1056,7 +1056,7 @@ bool tropic_data_multi_read(uint16_t first_slot, uint16_t slot_count,
// length.
size_t copy_length = min(slot_length - prefix_length, out_length - out_pos);
if (out_pos + copy_length > max_data_length) {
- false;
+ return false;
}
memcpy(&data[out_pos], &slot_buffer[prefix_length], copy_length);
out_pos += copy_length;
Why this scored 59/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.