fix(rust/trezor-thp): check `payload_len` is not less than `CHECKSUM_LEN`
What the developer wrote
Authored by M1nd3r
fix(rust/trezor-thp): check `payload_len` is not less than `CHECKSUM_LEN`
[no changelog]
Co-authored-by: mmilata <85857+mmilata@users.noreply.github.com>
What changed, and why it matters
This commit fixes a bug in the Trezor hardware wallet's Rust-based THP (Trezor Host Protocol) code where a received message could claim a payload shorter than the checksum length. Before the fix, this caused an integer underflow when calculating how many bytes to feed into the checksum, which could crash or confuse the device. The patch adds an explicit length check and uses a safer subtraction method.
Treat as a security-relevant hardening fix. Ensure the patched firmware is shipped and verify that all protocol length fields are similarly validated. No immediate external advisory is required unless a reproducible crash or exploit is demonstrated, but the fix should be included in release notes.
Security signals we found
Integer underflow in length calculation
Missing bounds check on parsed length field
Network/protocol input used in arithmetic without validation
Crash/DoS potential in packet reassembly
Fix includes regression tests
Evidence from the diff
In rust/trezor-thp/src/fragment.rs, payload_len - CHECKSUM_LEN was replaced with payload_len.saturating_sub(CHECKSUM_LEN) to prevent underflow. In rust/trezor-thp/src/header.rs, a new validation rule rejects headers where payload_len < CHECKSUM_LEN with Error::OutOfBounds. Tests were added for both the header parser and the reassembler. The issue is in the protocol packet reassembly path of the Trezor firmware’s Rust THP stack.
Changed components
rust/trezor-thp/src/fragment.rsrust/trezor-thp/src/header.rsTrezor Host Protocol (THP) reassemblerTrezor firmware Rust stackInspect captured patch +14 / −1
diff --git a/rust/trezor-thp/src/fragment.rs b/rust/trezor-thp/src/fragment.rs
index 044c1415..017d1445 100644
--- a/rust/trezor-thp/src/fragment.rs
+++ b/rust/trezor-thp/src/fragment.rs
@@ -125,7 +125,7 @@ impl<R: Role> Reassembler<R> {
let nbytes = after_header.len(); // Header::parse strips padding
buffer[..nbytes].copy_from_slice(after_header);
- let checksum_bytes = (payload_len - CHECKSUM_LEN).min(nbytes);
+ let checksum_bytes = payload_len.saturating_sub(CHECKSUM_LEN).min(nbytes);
checksum.update(&after_header[..checksum_bytes]);
Ok(Self {
@@ -367,4 +367,12 @@ mod test {
.zip(EVEN_LONGER_PAYLOADS_EXPECTED)
.for_each(|(got, expected)| assert_eq!(&hex::encode(got), expected));
}
+
+ #[test]
+ fn test_reassemble_shorter_than_checksum() {
+ let packet = &[0x04, 0x12, 0x34, 0x00, 0x03, 0x00, 0x00, 0x00];
+ let mut received = [0u8; MAX_MESSAGE];
+ let reassembler = Reassembler::<Device>::new(packet, &mut received);
+ assert!(matches!(reassembler, Err(Error::OutOfBounds)));
+ }
}
diff --git a/rust/trezor-thp/src/header.rs b/rust/trezor-thp/src/header.rs
index f4fa57e5..274a4179 100644
--- a/rust/trezor-thp/src/header.rs
+++ b/rust/trezor-thp/src/header.rs
@@ -111,6 +111,10 @@ impl<R: Role> Header<R> {
log::error!("Payload length exceeds {}.", MAX_PAYLOAD_LEN);
return Err(Error::OutOfBounds);
}
+ if payload_len < CHECKSUM_LEN {
+ log::error!("Payload length is less than {}.", CHECKSUM_LEN);
+ return Err(Error::OutOfBounds);
+ }
// strip padding if there is any
let without_padding = rest.len().min(payload_len.into());
let rest = &rest[..without_padding];
@@ -673,6 +677,7 @@ mod test {
"04fff00001", // bad channel id
"041111ffee", // invalid length field
"80fffe0000", // bad channel id
+ "0400010003", // payload_len < CHECKSUM_LEN
];
#[test]
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.