feat(erc7730): D-I iteration and signer-constant intent check

keepkeylib/erc7730_compiler.py

@@ -978,8 +978,9 @@ def depth(node):
 # together with the firmware table.
 _PATH, _LITERAL, _STRING = frozenset((1,)), frozenset((2,)), frozenset((3,))
 DEVICE_CAPABILITIES = {
-    # 2 and 3 (interpolated intent) only as one run directly after the intent
-    "display_opcodes": frozenset((1, 2, 3, 4, 5, 6, 10)),
+    # 2 and 3 (interpolated intent) only as one run directly after the intent;
+    # 5/6 groups; 7/8 one iteration at a time, calldata only
+    "display_opcodes": frozenset((1, 2, 3, 4, 5, 6, 7, 8, 10)),
     # formatter kind -> (argument role -> permitted sources, required roles)
     "formatters": {
         1: ({1: _PATH}, frozenset((1,))),                        # raw
@@ -997,8 +998,10 @@ def depth(node):
     "path_sources": frozenset((1, 2, 3)),
     # @.from, @.to and @.value, calldata definitions only
     "containers": frozenset((1, 2, 3)),
-    "path_step_opcodes": frozenset((1,)),
-    "condition_opcodes": frozenset((3,)),  # optional: always shown
+    # 2: every element, bound to the iteration's current element
+    "path_step_opcodes": frozenset((1, 2)),
+    # only "optional" (3), which is always shown
+    "condition_opcodes": frozenset((3,)),
     "conditions": True,
     "alias_set_max": 4,
     "enum_max": 16,
@@ -1063,24 +1066,38 @@ def _program_sections(program):
 
 def _abi_walk(nodes, steps):
     """Mirror of the firmware's preload walk; steps are (opcode, index).
-    Returns (refusal, leaf kind)."""
+    Returns (refusal, leaf kind or 0 for an iteration path, [] array node)."""
     node = 0
+    indexed = False
+    array = None
     for opcode, index in steps:
         if node >= len(nodes):
-            return "path leaves the ABI", 0
+            return "path leaves the ABI", 0, None
         kind, _, first, count, length = nodes[node]
         if kind == 8 and opcode == 1 and count and 0 <= index < count:
             node = first + index
         elif kind == 9 and opcode in (1, 2):
+            if opcode == 2:
+                if indexed:
+                    return "path indexes an array before iterating", 0, None
+                array = node
+            else:
+                indexed = True
             limit = MAX_ARRAY_ELEMENTS if length == ABSENT else length
             if not -limit <= index < limit:
-                return "path indexes beyond the array", 0
+                return "path indexes beyond the array", 0, None
             node = first
         else:
-            return "path does not name an ABI member", 0
-    if node >= len(nodes) or nodes[node][0] > 7:
-        return "path does not end at a value", 0
-    return None, nodes[node][0]
+            return "path does not name an ABI member", 0, None
+    if node >= len(nodes):
+        return "path does not end at a value", 0, None
+    if steps and steps[-1][0] == 2:
+        # An iteration path: a value too when its element is a leaf.
+        leaf = nodes[node][0]
+        return None, (leaf if leaf <= 7 else 0), array
+    if nodes[node][0] > 7:
+        return "path does not end at a value", 0, None
+    return None, nodes[node][0], array
 
 
 def device_refusal(program, capabilities=DEVICE_CAPABILITIES):
@@ -1157,6 +1174,8 @@ def literal_class(index):
 
     calldata = program[7] == 1
     path_classes = []
+    path_arrays = []
+    iterable = set()
     paths = sections.get(3, b"\0\0")
     at = 2
     for _ in range(u16(paths, 0)):
@@ -1172,6 +1191,7 @@ def literal_class(index):
         path_classes.append(
             (CLASS_UINT if index == 3 else CLASS_ADDRESS) if source == 2 else
             ("literal", index) if source == 3 else None)
+        path_arrays.append(None)
         steps = []
         for _ in range(count):
             opcode = paths[at]
@@ -1187,23 +1207,30 @@ def literal_class(index):
                 flags = paths[at]
                 at += 1 + 4 * bin(flags).count("1")
         if source == 1:
-            reason, leaf = _abi_walk(nodes, steps)
+            if sum(1 for step in steps if step[0] == 2) > 1:
+                return "path iterates more than once"
+            reason, leaf, array = _abi_walk(nodes, steps)
             if reason:
                 return reason
             path_classes[-1] = leaf
+            path_arrays[-1] = array
+            if steps and steps[-1][0] == 2:
+                iterable.add(len(path_classes) - 1)
 
     conditions = sections.get(5, b"\0\0")
     if u16(conditions, 0) and not capabilities["conditions"]:
         return "display conditions are not executed"
     for i in range(u16(conditions, 0)):
-        if conditions[2 + 8 * i] not in capabilities.get("condition_opcodes", ()):
+        if conditions[2 + 8 * i] not in capabilities.get("condition_opcodes",
+                                                         ()):
             return "condition opcode %d is not executed" % conditions[2 + 8 * i]
 
     formatters = sections.get(6, b"\0\0")
-    formatter_constants = []
+    formatter_arrays = []
     at = 2
     for _ in range(u16(formatters, 0)):
-        value_literal = False
+        value_array, any_array, mixed_arrays, value_literal = (
+            None, False, False, False)
         kind, argc = formatters[at], formatters[at + 2]
         at += 3
         if kind not in capabilities["formatters"]:
@@ -1238,13 +1265,23 @@ def literal_class(index):
                 if kind != 1:
                     return "only a raw field may show a signer constant"
                 value_literal = True
+            if source == 1:
+                argument_array = path_arrays[index]
+                if argument_array is not None:
+                    any_array = True
+                if role == 1:
+                    value_array = argument_array
+                elif argument_array != value_array:
+                    mixed_arrays = True
             seen.add(role)
         if not required <= seen:
             return "formatter kind %d lacks a required argument" % kind
-        formatter_constants.append(value_literal)
+        formatter_arrays.append((value_array, any_array, mixed_arrays,
+                                 value_literal))
 
     displays = sections.get(7, b"\0\0")
     run_closed = False
+    iteration = None
     for pc in range(u16(displays, 0)):
         opcode, _, a, b, c = struct.unpack(
             ">BBHHH", displays[2 + 8 * pc:10 + 8 * pc])
@@ -1259,10 +1296,24 @@ def literal_class(index):
             run_closed = True
         if opcode == 4 and c != ABSENT and not capabilities["conditions"]:
             return "display conditions are not executed"
-        if opcode == 3 and a < len(formatter_constants) and formatter_constants[a]:
+        if opcode == 3 and a < len(formatter_arrays) and formatter_arrays[a][3]:
             return "a signer constant cannot be an intent value"
         if opcode == 4 and a not in short_strings:
             return "a field label is longer than the device shows"
+        if opcode == 7:
+            if a not in iterable or iteration is not None or not calldata:
+                return "iteration is not executed here"
+            iteration = path_arrays[a]
+        elif opcode == 8:
+            iteration = None
+        elif opcode in (3, 4):
+            value_array, any_array, mixed_arrays, _ = (
+                formatter_arrays[a if opcode == 3 else b])
+            if any_array and iteration is None:
+                return "an iterating value outside an iteration"
+            if iteration is not None and (value_array != iteration or
+                                          mixed_arrays):
+                return "a field reads another array than its iteration"
     return None
 
 

tests/test_erc7730_compiler.py

@@ -260,8 +260,7 @@ def test_compiles_array_iteration_separator_and_optional_visibility():
     end = display[26:34]
     assert int.from_bytes(begin[6:8], "big") == 3
     assert int.from_bytes(end[2:4], "big") == 1
-    _firmware_validate(compiled,
-                       "path step opcode 2 is not executed")
+    _firmware_validate(compiled)
 
 
 def test_refuses_nested_array_iteration_the_device_cannot_verify():
@@ -418,8 +417,7 @@ def test_loads_bounded_includes_and_compiles_array_backed_group(tmp_path):
     count = int.from_bytes(display[:2], "big")
     opcodes = [display[2 + i * 8] for i in range(count)]
     assert opcodes == [1, 7, 5, 4, 4, 6, 8, 10]
-    _firmware_validate(compiled,
-                       "path step opcode 2 is not executed")
+    _firmware_validate(compiled)
 
 
 DEVICE_LIMITS = (
@@ -437,8 +435,10 @@ def test_loads_bounded_includes_and_compiles_array_backed_group(tmp_path):
 # amount, rather than reinterpret bytes the calldata does not say are one.
 # Phase B adds the interpolated intent, shown as numbered parts: 954.
 # Phase C adds amount, nftName, date, duration, unit, enum and @.value: 1138.
-# D-G enables groups and always-visible optional fields, without iteration.
-REGISTRY_SIGNABLE = 1221
+# Phase D adds groups, single-array iteration and "optional" fields. The
+# reviewed array-binding rule refuses 21 registry programs whose auxiliary
+# formatter paths do not walk the displayed item's array.
+REGISTRY_SIGNABLE = 1273
 
 
 def test_official_registry_all_calldata_formats_reach_firmware():
@@ -555,6 +555,62 @@ def test_signed_enum_keys_are_minimal_twos_complement():
         _firmware_validate(compile_calldata(descriptor, signature, 1,
                                             "0x" + "11" * 20), None)
 
+def test_refuses_scalar_value_repeated_inside_iteration():
+    descriptor = {"display": {"formats": {
+        "batch(address[] recipients,address fallback)": {
+            "intent": "Batch transfer",
+            "fields": [
+                {"path": "recipients.[]", "label": "Recipient",
+                 "format": "addressName", "separator": "Next recipient"},
+                {"path": "fallback", "label": "Fallback",
+                 "format": "addressName"},
+            ],
+        }
+    }}}
+    program = bytearray(_unchecked(
+        compile_calldata, descriptor,
+        "batch(address[] recipients,address fallback)", 1,
+        "0x1111111111111111111111111111111111111111"))
+    _firmware_validate(bytes(program))
+    # Replace the iterated formatter's path with the scalar formatter's path.
+    # The display still contains an iteration, so the device must refuse it.
+    offset = HEADER_SIZE
+    while offset < len(program):
+        kind = program[offset]
+        length = struct.unpack_from(">I", program, offset + 1)[0]
+        if kind == 6:
+            start = offset + 5
+            assert struct.unpack_from(">H", program, start)[0] == 2
+            assert program[start + 7:start + 9] != program[start + 14:start + 16]
+            program[start + 7:start + 9] = program[start + 14:start + 16]
+            break
+        offset += 5 + length
+    else:
+        pytest.fail("formatter section missing")
+    _firmware_validate(bytes(program),
+                       "a field reads another array than its iteration")
+
+
+def test_iteration_binds_every_formatter_path_to_its_array():
+    signature = (
+        "batch((uint256 amount,address token)[] items,address[] other,"
+        "address fallback)")
+    for token_path, refusal in (
+            ("items.[].token", None),
+            ("other.[]", "a field reads another array than its iteration"),
+            ("fallback", "a field reads another array than its iteration")):
+        descriptor = {"display": {"formats": {signature: {
+            "intent": "Batch", "fields": [{
+                "path": "items.[].amount", "label": "Amount",
+                "format": "tokenAmount",
+                "params": {"tokenPath": token_path}}]}}}}
+        program = _unchecked(compile_calldata, descriptor, signature, 1,
+                             "0x" + "11" * 20)
+        _firmware_validate(program, refusal)
+        if refusal:
+            with pytest.raises(DeviceCannotExecute, match=refusal):
+                compile_calldata(descriptor, signature, 1, "0x" + "11" * 20)
+
 
 def test_signer_constant_is_not_an_intent_value():
     signature = "pay(address recipient)"