refactor(rust/trezor-thp): error logging
What changed, and why it matters
This commit only adds error log messages to an internal Rust library (trezor-thp) used by Trezor hardware wallets. It does not change any security logic, parsing rules, or behavior. The same error conditions were already being returned before; now they also print a short diagnostic message. There is no indication this fixes or introduces a security vulnerability.
No security action required. Review as normal code-quality/logging change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change adds the log crate as a dependency and inserts log::error!(...) calls at existing error-return paths in fragment.rs and header.rs. All control flow, error variants, and validation thresholds remain identical. No new dependencies with known security issues are introduced beyond a standard logging crate, and the commit is tagged [no changelog] as a refactoring.
Changed components
rust/trezor-thp/src/fragment.rsrust/trezor-thp/src/header.rsrust/trezor-thp/Cargo.tomlInspect captured patch +12 / −1
diff --git a/rust/trezor-thp/Cargo.toml b/rust/trezor-thp/Cargo.toml
index d070e492..7aa4da0c 100644
--- a/rust/trezor-thp/Cargo.toml
+++ b/rust/trezor-thp/Cargo.toml
@@ -4,6 +4,7 @@ version = "0.1.0"
edition = "2024"
[dependencies]
+log = "0.4.29"
[dev-dependencies]
heapless = { version = "0.9.2", default-features = false }
diff --git a/rust/trezor-thp/src/fragment.rs b/rust/trezor-thp/src/fragment.rs
index 37ee8a22..f51850ec 100644
--- a/rust/trezor-thp/src/fragment.rs
+++ b/rust/trezor-thp/src/fragment.rs
@@ -132,10 +132,12 @@ impl<R: Role> Reassembler<R> {
pub fn update(&mut self, input: &[u8], buffer: &mut [u8]) -> Result<()> {
let (header, after_header) = Header::<R>::parse(input)?;
if !header.is_continuation() {
+ log::error!("Unexpected continuation.");
return Err(Error::UnexpectedInput);
}
if header.channel_id() != self.header.channel_id() {
+ log::error!("Unexpected channel id.");
return Err(Error::OutOfBounds);
}
@@ -185,6 +187,7 @@ impl<R: Role> Reassembler<R> {
pub fn single<'a>(buffer: &[u8], dest: &'a mut [u8]) -> Result<(Header<R>, &'a [u8])> {
let reassembler = Self::new(buffer, dest)?;
if !reassembler.is_done() {
+ log::error!("Single packet message expected.");
return Err(Error::MalformedData);
}
let reply_len = reassembler.verify(dest)?;
diff --git a/rust/trezor-thp/src/header.rs b/rust/trezor-thp/src/header.rs
index 1195d217..bd613b65 100644
--- a/rust/trezor-thp/src/header.rs
+++ b/rust/trezor-thp/src/header.rs
@@ -76,7 +76,10 @@ impl<R: Role> Header<R> {
/// Parse header from a byte slice. Return remaining subslice on success.
/// Note: sync bits are discarded and need to be obtained from input buffer separately.
pub fn parse(buffer: &[u8]) -> Result<(Self, &[u8])> {
- let (first_byte, rest) = buffer.split_first().ok_or(Error::MalformedData)?;
+ let Some((first_byte, rest)) = buffer.split_first() else {
+ log::error!("Packet too short.");
+ return Err(Error::MalformedData);
+ };
let cb = ControlByte::from(*first_byte);
if cb.is_codec_v1() {
if R::is_host() {
@@ -88,6 +91,7 @@ impl<R: Role> Header<R> {
}
let (channel_id, rest) = parse_u16(rest)?;
if !channel_id_valid(channel_id) {
+ log::error!("Invalid channel id {}.", channel_id);
return Err(Error::OutOfBounds);
}
if cb.is_continuation() {
@@ -95,6 +99,7 @@ impl<R: Role> Header<R> {
}
let (payload_len, rest) = parse_u16(rest)?;
if payload_len > MAX_PAYLOAD_LEN {
+ log::error!("Payload length exceeds {}.", MAX_PAYLOAD_LEN);
return Err(Error::OutOfBounds);
}
// strip padding if there is any
@@ -126,6 +131,7 @@ impl<R: Role> Header<R> {
{
return Ok((Self::ChannelAllocationResponse { payload_len }, rest));
}
+ log::error!("Unknown control byte {}.", u8::from(cb));
Err(Error::MalformedData)
}
@@ -153,6 +159,7 @@ impl<R: Role> Header<R> {
if res.payload_len() == payload_len {
Ok(Some(res))
} else {
+ log::error!("Unexpected payload length.");
Err(Error::MalformedData)
}
}
Why this scored 15/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.