diff --git a/keepkeylib/erc7730_compiler.py b/keepkeylib/erc7730_compiler.py index c6bad2a6..8552fb98 100644 --- a/keepkeylib/erc7730_compiler.py +++ b/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, 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,11 @@ 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,)), - "conditions": False, + # 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, "signer_text_max": 64, @@ -1062,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): @@ -1156,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)): @@ -1171,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] @@ -1186,20 +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", + ()): + 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"]: @@ -1234,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]) @@ -1255,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][2]: 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 diff --git a/tests/test_erc7730_compiler.py b/tests/test_erc7730_compiler.py index e48a80fc..b2ca75de 100644 --- a/tests/test_erc7730_compiler.py +++ b/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(): @@ -325,7 +324,7 @@ def test_compiles_typed_if_not_in_and_must_match_conditions(): literals = sections[4] assert int.from_bytes(literals[:2], "big") == 5 _firmware_validate(compiled, - "display conditions are not executed") + "condition opcode 7 is not executed") def test_compiles_interpolated_intent_and_metadata_enum(): @@ -383,8 +382,7 @@ def test_compiles_nested_field_group_with_balanced_links(): assert [item[0] for item in instructions] == [1, 5, 4, 4, 6, 10] assert int.from_bytes(instructions[1][6:8], "big") == 4 assert int.from_bytes(instructions[4][2:4], "big") == 1 - _firmware_validate(compiled, - "display opcode 5 is not executed") + _firmware_validate(compiled) def test_loads_bounded_includes_and_compiles_array_backed_group(tmp_path): @@ -419,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 = ( @@ -438,7 +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. -REGISTRY_SIGNABLE = 1138 +# 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(): @@ -554,3 +554,59 @@ def test_signed_enum_keys_are_minimal_twos_complement(): "params": {"$ref": "$.metadata.enums.side"}}]}}}} _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)