chore(ethereum): enforce context-based parser type
What the developer wrote
Authored by Ioan Bizău
chore(ethereum): enforce context-based parser type
Only allow dynamic types to be parsed in a Dynamic context and
non-Dynamic in an Atomic context.
[no changelog]
What changed, and why it matters
This commit tightens validation in Trezor's Ethereum 'clear signing' feature. It now rejects cases where a host computer tells the device to parse a variable-length type (like a string or byte blob) as if it were a fixed 32-byte value, or vice versa. This reduces the chance that a malicious or buggy host could trick the device into misreading transaction data, but the commit itself does not claim to fix a specific known attack.
Treat as a defensive hardening patch. Review whether the prior permissive parser selection could have led to incorrect clear-signing displays or host-device state desynchronization, and consider adding regression tests for mixed-context ABI type definitions. No urgent user action is indicated by the commit alone.
Security signals we found
Input validation hardening for host-supplied ABI type descriptors
Context-aware parser selection prevents type confusion between atomic and dynamic ABI values
Potential UI/display security impact if wrong parser had been used for transaction fields
No explicit security claim or CVE in commit message
Evidence from the diff
The change modifies _get_parser() in core/src/apps/ethereum/clear_signing.py to require the caller to specify whether the requested Ethereum ABI type is being used in a dynamic (variable-length) or atomic (32-byte) context. Dynamic types (ABI_BYTES, ABI_STRING) are only accepted when is_dynamic=True; atomic types are only accepted when is_dynamic=False. Previously the same type could be parsed in either context, which could allow a host-supplied format definition to use a dynamic parser for an atomic slot or an atomic parser for a dynamic slot, potentially causing incorrect decoding of values shown to the user during clear signing.
Changed components
Trezor firmware Ethereum clear signing parsercore/src/apps/ethereum/clear_signing.pyEthereumABIValueInfo protobuf handlingInspect captured patch +17 / −12
diff --git a/core/src/apps/ethereum/clear_signing.py b/core/src/apps/ethereum/clear_signing.py
index 41a86154..bf15a317 100644
--- a/core/src/apps/ethereum/clear_signing.py
+++ b/core/src/apps/ethereum/clear_signing.py
@@ -165,16 +165,21 @@ def parse_uint256_array(raw_data: memoryview) -> list[Value]:
DYNAMIC_DATA_PARSERS = [parse_bytes, parse_string, parse_uint256_array]
-def _get_parser(t: int) -> Parser:
- """Get a parser for a type we received over the wire protocol."""
+def _get_parser(t: int, is_dynamic: bool) -> Parser:
+ """Get a parser for a type we received over the wire protocol.
+ `is_dynamic` selects whether the type is being used in a dynamic
+ (variable-length) or atomic (32-byte) context, and must match the type."""
from trezor.enums import EthereumABIType as T
+ if is_dynamic:
+ if t == T.ABI_BYTES:
+ return parse_bytes
+ elif t == T.ABI_STRING:
+ return parse_string
+ raise InvalidFormatDefinition
+
if t == T.ABI_ADDRESS:
return parse_address
- elif t == T.ABI_BYTES:
- return parse_bytes
- elif t == T.ABI_STRING:
- return parse_string
elif t == T.ABI_UINT256:
return parse_uint256
elif t == T.ABI_UINT248:
@@ -213,9 +218,9 @@ def _get_parser(t: int) -> Parser:
def _get_leaf_parser(info: EthereumABIValueInfo) -> Parser:
"""Get a parser for a leaf (atomic or dynamic) value. Raises for nested structures."""
if info.atomic is not None:
- return _get_parser(info.atomic)
+ return _get_parser(info.atomic, is_dynamic=False)
elif info.dynamic is not None:
- return _get_parser(info.dynamic)
+ return _get_parser(info.dynamic, is_dynamic=True)
raise InvalidFormatDefinition
@@ -408,9 +413,9 @@ class ABIValue:
@staticmethod
def from_proto(info: EthereumABIValueInfo) -> "ABIValue":
if info.atomic is not None:
- return Atomic(_get_parser(info.atomic))
+ return Atomic(_get_parser(info.atomic, is_dynamic=False))
elif info.dynamic is not None:
- return Dynamic(_get_parser(info.dynamic))
+ return Dynamic(_get_parser(info.dynamic, is_dynamic=True))
elif info.tuple is not None:
return Tuple(
tuple(_get_leaf_parser(f) for f in info.tuple.fields),
@@ -419,9 +424,9 @@ class ABIValue:
elif info.array is not None:
element = info.array
if element.atomic is not None:
- return Array(Atomic(_get_parser(element.atomic)))
+ return Array(Atomic(_get_parser(element.atomic, is_dynamic=False)))
elif element.dynamic is not None:
- return Array(Dynamic(_get_parser(element.dynamic)))
+ return Array(Dynamic(_get_parser(element.dynamic, is_dynamic=True)))
elif element.tuple is not None:
return Array(
Tuple(
Why this scored 46/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.