feat(clear_signing): support deeper structures.
What changed, and why it matters
This commit expands the Ethereum clear-signing parser in Trezor firmware to support nested tuples and arrays, while adding explicit safety limits on recursion depth and array nesting. The change itself is defensive: it replaces a hard-coded shallow decoder with a recursive one that caps how deep and how many array layers it will accept. The README explains why nested arrays are dangerous—an attacker can craft small transaction data that expands into a huge number of values in memory, potentially exhausting the device. The commit therefore reduces the risk of denial-of-service or memory-exhaustion attacks during transaction decoding, rather than introducing a new vulnerability.
Treat this as a security-hardening change. Review that _MAX_ABI_NESTING and _MAX_NESTED_ARRAYS are enforced in all code paths, including any future parser variants, and confirm the bounds match device memory and worst-case decode-cost budgets. Consider adding a changelog or advisory note because the change is security-relevant.
Security signals we found
Adds explicit recursion and array-nesting caps to an Ethereum ABI parser
README documents a known amplification/aliasing risk in nested dynamic arrays
Replaces a shallow, hard-coded decoder with a bounded recursive decoder
Prevents unbounded parse trees that could exhaust device memory
No changelog entry despite security-relevant behavioral change
Evidence from the diff
The patch refactors ABIValue.from_proto in core/src/apps/ethereum/clear_signing.py from a hand-unrolled, depth-limited builder into a recursive builder guarded by _MAX_ABI_NESTING (8) and _MAX_NESTED_ARRAYS (2). It removes the old _get_leaf_value helper and the explicit rejection of tuple-in-tuple and tuple-in-array. The README adds a ‘Nesting limits’ section explaining that nested dynamic arrays can alias element bodies, causing calldata size to grow linearly while decoded values grow multiplicatively. Tests are updated to assert that tuple-in-tuple and tuple-in-array now parse correctly, and that three nested arrays remain rejected by the new limit rather than by missing cases.
Changed components
core/src/apps/ethereum/clear_signing.pycore/src/apps/ethereum/README.mdcore/tests/test_apps.ethereum.clear_signing.pyInspect captured patch +101 / −49
### core/src/apps/ethereum/README.md
@@ -157,3 +157,42 @@ with `head_size == 0` inside an array would defeat the heads bounds pre-check
(`array_length * 0`), letting an attacker-controlled length word drive an
unbounded parse loop. Rejecting them guarantees `head_size >= 32` for every
constructible type.
+
+## Nesting limits
+
+`ABIValue.from_proto` bounds two things on every root-to-leaf path of the
+type tree: total recursion depth (`_MAX_ABI_NESTING`) and the number of
+`Array` levels (`_MAX_NESTED_ARRAYS`). The array bound is the one that
+matters for parse cost.
+
+Array lengths come from calldata and element bodies can alias, so nested
+arrays multiply; tuple fan-out is bounded by the definition. Nothing in the
+encoding requires the offsets of a dynamic array's elements to be distinct,
+so every element of an outer array may point at the same inner body. Take
+`uint256[][]` as the only parameter:
+
+```text
+w0 0x00 ...0020 offset of outer array body
+w1 0x20 ...0003 outer element count = 3
+w2 0x40 ...0060 [0] -> 0x40 + 0x60 = 0xa0 ┐
+w3 0x60 ...0060 [1] -> 0xa0 │ all three alias
+w4 0x80 ...0060 [2] -> 0xa0 ┘ the same body
+w5 0xa0 ...0003 inner element count = 3
+w6 0xc0 ...0001
+w7 0xe0 ...0002
+w8 0x100 ...0003
+```
+
+9 words decode to `[[1,2,3],[1,2,3],[1,2,3]]`: 9 values, where an honest
+encoding would have needed 17 words. With `N` outer and `M` inner elements
+the calldata grows as `N + M` but the decoded output as `N × M`; each further
+array level multiplies again. Within `_MAX_DATA_STORED` (6144 bytes, 192
+words) that is ~190 values for one level, ~9,000 for two, and ~250,000 for
+three - beyond what the device can hold in memory.
+
+Tuples don't need the same treatment: a tuple's fan-out is its field count,
+fixed by the definition, and when a dynamic tuple's body is aliased by many
+array elements that multiplication is the array's, already counted. Arrays of
+static elements can't alias at all - they are laid out in place, and the heads
+bounds pre-check (`array_length × head_size`) keeps their element count within
+the calldata size.
### core/src/apps/ethereum/clear_signing.py
@@ -61,6 +61,12 @@
_EVM_WORD_BITS = const(8 * _EVM_WORD_SIZE)
_ADDRESS_BYTES = const(20)
+# To be kept in sync in the definitions repo (definitions/ethereum/erc7730/erc7730.py)
+# Recursion depth for raw decoding (ABIValue.from_proto)
+_MAX_ABI_NESTING = const(8)
+# Encoding depth counting just array nesting. To be increased cautiously.
+_MAX_NESTED_ARRAYS = const(2)
+
class ClearSigningFailed(Exception):
pass
@@ -646,40 +652,29 @@ def parse_body(self, raw_data: memoryview, pos: int) -> AnyValue:
raise NotImplementedError
@staticmethod
- def from_proto(info: EthereumABIValueInfo) -> "ABIValue":
+ def from_proto(
+ info: EthereumABIValueInfo, depth: int = 0, arrays: int = 0
+ ) -> "ABIValue":
+ """Build the parser tree for one wire descriptor node.
+
+ `depth` bounds recursion; `arrays` counts only the
+ `Array` levels on this "root-to-leaf" path.
+ Array lengths come from calldata and element bodies can alias, so nested
+ arrays multiply; tuple fan-out is bounded by the definition."""
+ if depth > _MAX_ABI_NESTING or arrays > _MAX_NESTED_ARRAYS:
+ raise InvalidFormatDefinition
if info.atomic is not None:
return Atomic(_get_parser(info.atomic, is_dynamic=False))
elif info.dynamic is not None:
return DynamicLeaf(_get_parser(info.dynamic, is_dynamic=True))
elif info.tuple is not None:
return Tuple(
- tuple(_get_leaf_value(f) for f in info.tuple.fields),
+ tuple(
+ ABIValue.from_proto(f, depth + 1, arrays) for f in info.tuple.fields
+ )
)
elif info.array is not None:
- element = info.array
- if element.atomic is not None:
- return Array(Atomic(_get_parser(element.atomic, is_dynamic=False)))
- elif element.dynamic is not None:
- return Array(DynamicLeaf(_get_parser(element.dynamic, is_dynamic=True)))
- elif element.tuple is not None:
- fields = tuple(_get_leaf_value(f) for f in element.tuple.fields)
- # A non-array (leaf) struct/tuple is dynamic if any of its fields is dynamic.
- # E.g. of dynamic members: bytes, string, uint256[], bytes[], bytes[][] etc.
- # An array (this outer structure) is always* dynamic regardless of its fields.
- # (*Unless it's of fixed length, which generally don't exist.)
- return Array(Tuple(fields))
- elif element.array is not None:
- inner = element.array
- if inner.atomic is not None:
- return Array(
- Array(Atomic(_get_parser(inner.atomic, is_dynamic=False)))
- )
- elif inner.dynamic is not None:
- return Array(
- Array(DynamicLeaf(_get_parser(inner.dynamic, is_dynamic=True)))
- )
- raise InvalidFormatDefinition # deeper nesting not supported
- raise InvalidFormatDefinition
+ return Array(ABIValue.from_proto(info.array, depth + 1, arrays + 1))
raise InvalidFormatDefinition
@@ -707,15 +702,6 @@ def _read_dynamic_data(raw_data: memoryview, pointer: int) -> memoryview:
return raw_data[body_start : body_start + length]
-def _get_leaf_value(info: EthereumABIValueInfo) -> ABIValue:
- """Build a leaf (atomic or dynamic) node. Raises for nested structures."""
- if info.atomic is not None:
- return Atomic(_get_parser(info.atomic, is_dynamic=False))
- elif info.dynamic is not None:
- return DynamicLeaf(_get_parser(info.dynamic, is_dynamic=True))
- raise InvalidFormatDefinition
-
-
class DynamicLeaf(ABIValue):
"""`strings` or `bytes`. Their body is a
one-word byte length followed by that many bytes of data."""
### core/tests/test_apps.ethereum.clear_signing.py
@@ -1823,6 +1823,44 @@ def test_array_of_dynamic_tuples(self):
self.assertEqual(parsed, [(self.ADDR1, one), (self.ADDR2, two)])
self.assertEqual(consumed, 32)
+ # --- non-leaf tuple fields ---
+
+ def test_tuple_field_may_be_a_tuple(self):
+ # ((uint256 b) inner): a struct as a struct member. Both are static,
+ # so the whole thing is one word encoded in place.
+ inner = p_tuple([p_atomic(EABIT.ABI_UINT256)], is_dynamic=False)
+ node = ABIValue.from_proto(p_tuple([inner], is_dynamic=False))
+
+ self.assertFalse(node.is_dynamic)
+ self.assertEqual(node.head_size, 32)
+
+ data = memoryview(FIVE_RANDOM_BYTES + to_bytes(42) + SEVEN_RANDOM_BYTES)
+ parsed, consumed = node.parse(data, len(FIVE_RANDOM_BYTES))
+
+ self.assertEqual(parsed, ((42,),))
+ self.assertEqual(consumed, 32)
+
+ def test_tuple_field_may_be_an_array(self):
+ # (uint256[] xs): the array member makes the struct dynamic, and the
+ # array's own offset is relative to the struct's body start.
+ inner = p_array(p_atomic(EABIT.ABI_UINT256))
+ node = ABIValue.from_proto(p_tuple([inner], is_dynamic=True))
+
+ self.assertTrue(node.is_dynamic)
+ self.assertEqual(node.head_size, 32)
+
+ data = memoryview(
+ to_bytes(32) # 0 outer head -> struct body at 32
+ + to_bytes(32) # 32 field 0 head, rel to 32 -> array body at 64
+ + to_bytes(2) # 64 element count
+ + to_bytes(7) # 96
+ + to_bytes(8) # 128
+ )
+ parsed, consumed = node.parse(data, 0)
+
+ self.assertEqual(parsed, ([7, 8],))
+ self.assertEqual(consumed, 32)
+
# --- shapes the decoder refuses ---
def test_no_variant_set_rejected(self):
@@ -1834,21 +1872,10 @@ def test_unknown_abi_type_rejected(self):
with self.assertRaises(InvalidFormatDefinition):
ABIValue.from_proto(p_atomic(99))
- def test_tuple_field_may_not_be_a_tuple(self):
- # Lifted in a later change; pinned here so that change is visible.
- inner = p_tuple([p_atomic(EABIT.ABI_UINT256)], is_dynamic=False)
- with self.assertRaises(InvalidFormatDefinition):
- ABIValue.from_proto(p_tuple([inner], is_dynamic=False))
-
- def test_tuple_field_may_not_be_an_array(self):
- # Lifted in a later change; pinned here so that change is visible.
- inner = p_array(p_atomic(EABIT.ABI_UINT256))
- with self.assertRaises(InvalidFormatDefinition):
- ABIValue.from_proto(p_tuple([inner], is_dynamic=True))
-
def test_three_nested_arrays_rejected(self):
- # Stays rejected in the later change, but for a stated nesting limit
- # rather than by falling off the end of the unrolled cases.
+ # Rejected by `_MAX_NESTED_ARRAYS`: each nested array level multiplies
+ # the parse work by an attacker-controlled element count, so the cap is
+ # what keeps the worst case at the order already reachable today.
node = p_array(p_array(p_array(p_atomic(EABIT.ABI_UINT256))))
with self.assertRaises(InvalidFormatDefinition):
ABIValue.from_proto(node)Why this scored 51/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.