fix(python): raise ProtocolError on invalid THP continuation
What changed, and why it matters
This commit fixes a small but real bug in the Python Trezor library's handling of USB/transport messages. When reassembling a multi-part message, if a follow-up chunk had a malformed header, the code would crash with a low-level Python struct.error instead of raising a proper ProtocolError. The fix wraps that parsing in a try/except and raises a clean ProtocolError with a clear message. It also slightly improves the error message for an invalid initial header.
Treat as a minor hardening fix. Review whether the struct.error could have caused higher-level code to abort uncleanly or leak unexpected exception types to callers. No urgent action beyond normal patch uptake is indicated.
Security signals we found
Unhandled struct.error on continuation header parsing
Improper exception type for malformed protocol data
Transport protocol parsing robustness improvement
No changelog entry supplied
Evidence from the diff
In python/src/trezorlib/thp/thp_io.py, read_and_assemble() parses an initial packet header with struct.unpack inside a try/except that already raises exceptions.ProtocolError. However, when reading continuation chunks, a second struct.unpack for FORMAT_STR_CONT was unprotected. A too-short or malformed continuation chunk would therefore propagate struct.error rather than the intended exceptions.ProtocolError. The patch adds a try/except around the continuation header unpack and raises exceptions.ProtocolError(‘Invalid continuation header’). It also rewords the initial-header error from ‘Invalid header’ to ‘Invalid message header’.
Changed components
python/src/trezorlib/thp/thp_io.pyTrezor Python client library THP transport layerInspect captured patch +7 / −4
diff --git a/python/src/trezorlib/thp/thp_io.py b/python/src/trezorlib/thp/thp_io.py
index 62c437dc..a929f80e 100644
--- a/python/src/trezorlib/thp/thp_io.py
+++ b/python/src/trezorlib/thp/thp_io.py
@@ -95,7 +95,7 @@ def read_and_assemble(transport: Transport, timeout: float | None = None) -> Mes
FORMAT_STR_INIT, chunk[:INIT_HEADER_LENGTH]
)
except struct.error:
- raise exceptions.ProtocolError("Invalid header")
+ raise exceptions.ProtocolError("Invalid message header")
if ctrl_byte == CONTINUATION_PACKET:
LOG.warning("Skipping unexpected continuation packet")
@@ -116,9 +116,12 @@ def read_and_assemble(transport: Transport, timeout: float | None = None) -> Mes
)
chunk = transport.read_chunk(timeout=timeout)
- ctrl_byte, cid = struct.unpack(
- FORMAT_STR_CONT, chunk[:CONT_HEADER_LENGTH]
- )
+ try:
+ ctrl_byte, cid = struct.unpack(
+ FORMAT_STR_CONT, chunk[:CONT_HEADER_LENGTH]
+ )
+ except struct.error:
+ raise exceptions.ProtocolError("Invalid continuation header")
if ctrl_byte != CONTINUATION_PACKET:
LOG.warning(
"Expected continuation, got: %s",
Why this scored 29/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.