fix(core/thp): correct `Failure_InvalidProtocol` constant message
What the developer wrote
Authored by Roman Zeyde
fix(core/thp): correct `Failure_InvalidProtocol` constant message
[no changelog]
What changed, and why it matters
This commit fixes a hard-coded error message sent by Trezor devices when an older protocol (v1) is used on a connection that expects the newer THP protocol. The message's length field was wrong (claimed 20 bytes instead of 2), which could cause the receiving host to read extra, unintended bytes. The fix corrects the length and adds a test to ensure the response is exactly the right size.
No immediate action required beyond normal patch review and merge. The change is a correctness fix; consider auditing other hard-coded wire-format constants for similar length mismatches.
Security signals we found
Hard-coded protocol response length field corrected
Test added to prevent trailing-byte regression
Failure_InvalidProtocol response generated on THP interface for legacy v1 packets
Evidence from the diff
In core/src/trezor/wire/thp/interface_context.py, the device constructs a raw Failure_InvalidProtocol response for legacy protocol-v1 packets received on a THP interface. The previous hard-coded byte string used \x00\x00\x00\x14 (msg_size = 20) for the Failure message length, but the actual payload is only 2 bytes (\x08\x11, field tag/length for code). The patch changes the length to \x00\x00\x00\x02. The test now also verifies that the encoded response matches the canonical protobuf encoding of messages.Failure(code=messages.FailureType.InvalidProtocol) with no trailing bytes.
Changed components
core/src/trezor/wire/thp/interface_context.pytests/device_tests/thp/test_basic.pyInspect captured patch +5 / −2
diff --git a/core/src/trezor/wire/thp/interface_context.py b/core/src/trezor/wire/thp/interface_context.py
index 70ae0d00..be20f385 100644
--- a/core/src/trezor/wire/thp/interface_context.py
+++ b/core/src/trezor/wire/thp/interface_context.py
@@ -172,7 +172,7 @@ class InterfaceContext:
response = bytearray(self._iface.TX_PACKET_LEN)
# Codec_v1 magic constant:
# "?##" + Failure message type + msg_size + msg_data (code = "Failure_InvalidProtocol")
- utils.memcpy(response, 0, b"?##\x00\x03\x00\x00\x00\x14\x08\x11", 0)
+ utils.memcpy(response, 0, b"?##\x00\x03\x00\x00\x00\x02\x08\x11", 0)
await self._write_packets([response])
async def _handle_broadcast(self, packet: AnyBytes) -> None:
diff --git a/tests/device_tests/thp/test_basic.py b/tests/device_tests/thp/test_basic.py
index 61b76306..f735b00f 100644
--- a/tests/device_tests/thp/test_basic.py
+++ b/tests/device_tests/thp/test_basic.py
@@ -24,8 +24,11 @@ def test_v1(client: Client):
# There should be a failure response to received init packet (starts with "?##")
write_padded(client.transport, b"?## Init packet")
res_id, res_data = protocol_v1.read(client.transport)
+ expected = messages.Failure(code=messages.FailureType.InvalidProtocol)
res = DEFAULT_MAPPING.decode(res_id, res_data)
- assert res == messages.Failure(code=messages.FailureType.InvalidProtocol)
+ assert res == expected
+ # make sure the constant "InvalidProtocol" response doesn't have trailing bytes (#6549)
+ assert (res_id, res_data) == DEFAULT_MAPPING.encode(expected)
# There should be no response for continuation packet (starts with "?" only)
write_padded(client.transport, b"? Cont packet")
Why this scored 21/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.