diff --git a/common/tests/fixtures/ethereum/sign_tx.json b/common/tests/fixtures/ethereum/sign_tx.json index e5cf5cb08..34ca455a0 100644 --- a/common/tests/fixtures/ethereum/sign_tx.json +++ b/common/tests/fixtures/ethereum/sign_tx.json @@ -5,7 +5,7 @@ }, "tests": [ { - "name": "erc20_token", + "name": "erc20_transfer", "parameters": { "comment": "Sending 291 Grzegorz Brzęczyszczykiewicz tokens to address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b", "data": "a9059cbb000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000123", @@ -24,6 +24,26 @@ "sig_s": "633a74429eb6d3aeec4ed797542236a85daab3cab15e37736b87a45697541d7a" } }, + { + "name": "erc20_approve", + "parameters": { + "comment": "Approving spending of 100 Grzegorz Brzęczyszczykiewicz tokens for address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b", + "data": "095ea7b3000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000064", + "path": "m/44'/60'/0'/0/0", + "to_address": "0xfc6b5d6af8a13258f7cbd0d39e11b35e01a32f93", + "chain_id": 1, + "nonce": "0x0", + "gas_price": "0x14", + "gas_limit": "0x14", + "tx_type": null, + "value": "0x0" + }, + "result": { + "sig_v": 37, + "sig_r": "b10bb751b48d72bbb6d55ecb34800c08f92fda818480f67190d923c5bd73d2c4", + "sig_s": "78fb7fa237bc6bfa0e5bdb96164db4162f41866f2f30b54295d947fe87a813a8" + } + }, { "name": "wanchain", "parameters": { diff --git a/common/tests/fixtures/ethereum/sign_tx_eip1559.json b/common/tests/fixtures/ethereum/sign_tx_eip1559.json index 72261153e..d716288e7 100644 --- a/common/tests/fixtures/ethereum/sign_tx_eip1559.json +++ b/common/tests/fixtures/ethereum/sign_tx_eip1559.json @@ -62,7 +62,7 @@ } }, { - "name": "erc20", + "name": "erc20_transfer", "parameters": { "comment": "Sending 291 Grzegorz Brzęczyszczykiewicz tokens to address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b", "data": "a9059cbb000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000123", @@ -81,6 +81,26 @@ "sig_s": "399bff8752539176c4b2f1d5d2a8f6029f79841d28802149ab339a033ffe4c1f" } }, + { + "name": "erc20_approve", + "parameters": { + "comment": "Approving spending of 100 Grzegorz Brzęczyszczykiewicz tokens for address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b", + "data": "095ea7b3000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000064", + "path": "m/44'/60'/0'/0/0", + "to_address": "0xfc6b5d6af8a13258f7cbd0d39e11b35e01a32f93", + "chain_id": 1, + "nonce": "0x0", + "gas_limit": "0x14", + "max_gas_fee": "0x14", + "max_priority_fee": "0x1", + "value": "0x0" + }, + "result": { + "sig_v": 0, + "sig_r": "e172298f9e5aeef170bfd9acff39249de3c7abbdc5637c135b2e82247e8ee12a", + "sig_s": "129cd1e0deaf4bd82e1064409188f1fadc8b7e6adfeb04ae4e407d5451ff43d8" + } + }, { "name": "large_chainid", "parameters": { diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index b8b0f5b1f..593278134 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -23,6 +23,17 @@ if TYPE_CHECKING: ) +async def require_confirm_smart_contract( + func_name: str | bytes, func_args: list[tuple[str, str | bytes]] +): + from trezor.ui.layouts import confirm_properties + + await confirm_properties( + "confirm_smart_contract", "Smart contract", [("Function:", func_name)] + ) + await confirm_properties("confirm_smart_contract", "Smart contract", func_args) + + async def require_confirm_tx( to_bytes: bytes, value: int, diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py index f843e7ef0..b0d5e288b 100644 --- a/core/src/apps/ethereum/sign_tx.py +++ b/core/src/apps/ethereum/sign_tx.py @@ -14,6 +14,7 @@ if TYPE_CHECKING: from .definitions import Definitions from .keychain import MsgInSignTx + from typing import Any # Maximum chain_id which returns the full signature_v (which must fit into an uint32). @@ -32,7 +33,6 @@ async def sign_tx( from trezor.utils import HashWriter from apps.common import paths - from .layout import require_confirm_data, require_confirm_tx # check @@ -44,16 +44,11 @@ async def sign_tx( await paths.validate_path(keychain, msg.address_n) - # Handle ERC20s - token, address_bytes, recipient, value = await handle_erc20(msg, defs) - - data_total = msg.data_length # local_cache_attribute - - if token is None and data_total > 0: - await require_confirm_data(msg.data_initial_chunk, data_total) + address_bytes = bytes_from_address(msg.to) + token, value = await sign_tx_common(msg, defs, address_bytes) await require_confirm_tx( - recipient, + address_bytes, value, int.from_bytes(msg.gas_price, "big"), int.from_bytes(msg.gas_limit, "big"), @@ -62,6 +57,7 @@ async def sign_tx( bool(msg.chunkify), ) + data_total = msg.data_length data = bytearray() data += msg.data_initial_chunk data_left = data_total - len(msg.data_initial_chunk) @@ -99,33 +95,136 @@ async def sign_tx( return result -async def handle_erc20( +async def sign_tx_common( msg: MsgInSignTx, definitions: Definitions, -) -> tuple[EthereumTokenInfo | None, bytes, bytes, int]: + address_bytes: bytes, +) -> tuple[EthereumTokenInfo | None, int]: + from .layout import ( + require_confirm_unknown_token, + require_confirm_smart_contract, + require_confirm_tx, + require_confirm_data, + ) from . import tokens - from .layout import require_confirm_unknown_token data_initial_chunk = msg.data_initial_chunk # local_cache_attribute token = None - address_bytes = recipient = bytes_from_address(msg.to) value = int.from_bytes(msg.value, "big") - if ( - len(msg.to) in (40, 42) - and len(msg.value) == 0 - and msg.data_length == 68 - and len(data_initial_chunk) == 68 - and data_initial_chunk[:16] - == b"\xa9\x05\x9c\xbb\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" - ): + if len(msg.to) in (40, 42) and value == 0: + # Smart Contract + func_name, func_args, transfer = _resolve_tx_data_field(data_initial_chunk) + # TODO handle ValueError from _resolve_tx_data_field, presently it fails `test_data_streaming` token = definitions.get_token(address_bytes) - recipient = data_initial_chunk[16:36] - value = int.from_bytes(data_initial_chunk[36:68], "big") - if token is tokens.UNKNOWN_TOKEN: await require_confirm_unknown_token(address_bytes) - - return token, address_bytes, recipient, value + if transfer[0]: + # 'transfer' functions should override the value + arg_val_idx = transfer[1] + value = int(func_args[arg_val_idx][1]) + else: + # we want to show default network at summary screen (i.e. 0 ETH) + token = None + await require_confirm_smart_contract(func_name, func_args) + else: + # Regular transaction + await require_confirm_tx(address_bytes, value, definitions.network, token) + if msg.data_length > 0: + await require_confirm_data(data_initial_chunk, msg.data_length) + + return token, value + + +def _resolve_type(val: memoryview, type_str: str) -> str: + from ubinascii import hexlify + from .helpers import address_from_bytes + + if type_str == "int": + return str(int.from_bytes(val, "big")) + elif type_str == "str": + # TODO improve shown text + return bytes(val).decode() + elif type_str == "bytes": + return hexlify(val).decode() + elif type_str == "address": + return address_from_bytes(val[-20:]) + else: + raise ValueError + + +FUNCTIONS_DEF: dict[bytes, dict[str, Any]] = { + b"\xa9\x05\x9c\xbb": { + "name": "transfer", + "args": [ + ("Recipient", "address"), + ("Amount", "int"), + ], + "transfer": (True, 1), + }, + b"\x09\x5e\xa7\xb3": { + "name": "approve", + "args": [ + ("Address", "address"), + ("Amount", "int"), + ], + "transfer": (False, 0), + }, + b"\x00\x00\x00\x42": { + "name": "args_test", + "args": [ + ("Arg0_int", "int"), + ("Arg1_str", "str"), + ("Arg2_bytes", "bytes"), + ("Arg3_address", "address"), + ], + "transfer": (False, 0), + # TODO token to address assignment is done by additional entry, where: + # - 1st value is the idx of the value of a token + # - 2nd value is the idx of the address of the token -> to be used in the `definitions.get_token(addr)` call + # - if 1st == 2nd: token is assigned based on contract address + # "token_assign": (0, 3), + }, +} + + +def _resolve_tx_data_field( + data_bytes: bytes, +) -> tuple[str, list[tuple[str, str | bytes]], tuple[bool, int]]: + from ubinascii import hexlify + + data_args_len = len(data_bytes) + N_BYTES_FUNC = 4 + N_BYTES_ARG = 32 + n_args = (data_args_len - N_BYTES_FUNC) // N_BYTES_ARG + data = memoryview(data_bytes) + + def _data_field_aligned(data_args_len: int, n_args: int) -> bool: + # checks if "Data" field doesn't have trailing bytes + return data_args_len == (n_args * N_BYTES_ARG + N_BYTES_FUNC) + + def _get_nth_arg(data_mv: memoryview, n: int) -> memoryview: + # returns slice of the nth argument in "Data" field + beg = (n + 0) * N_BYTES_ARG + N_BYTES_FUNC + end = (n + 1) * N_BYTES_ARG + N_BYTES_FUNC + return data_mv[beg:end] + + if data_args_len < N_BYTES_FUNC or not _data_field_aligned(data_args_len, n_args): + raise ValueError + + func_signature_bytes = data_bytes[:N_BYTES_FUNC] + func_def = FUNCTIONS_DEF.get(func_signature_bytes, None) + if func_def is not None and n_args == len(func_def["args"]): + func_name = func_def["name"] + func_args = [ + (f"{name}:", _resolve_type(_get_nth_arg(data, i), type_str)) + for i, (name, type_str) in enumerate(func_def["args"]) + ] + transfer = func_def["transfer"] + else: + func_name = hexlify(func_signature_bytes).decode() + func_args = [(f"Input {i}:", _get_nth_arg(data, i)) for i in range(n_args)] + transfer = (False, 0) + return (func_name, func_args, transfer) def _get_total_length(msg: EthereumSignTx, data_total: int) -> int: diff --git a/core/src/apps/ethereum/sign_tx_eip1559.py b/core/src/apps/ethereum/sign_tx_eip1559.py index d9954ad12..6d01e5ba2 100644 --- a/core/src/apps/ethereum/sign_tx_eip1559.py +++ b/core/src/apps/ethereum/sign_tx_eip1559.py @@ -41,9 +41,8 @@ async def sign_tx_eip1559( from trezor.utils import HashWriter from apps.common import paths - - from .layout import require_confirm_data, require_confirm_tx_eip1559 - from .sign_tx import check_common_fields, handle_erc20, send_request_chunk + from .layout import require_confirm_tx_eip1559 + from .sign_tx import sign_tx_common, send_request_chunk, check_common_fields gas_limit = msg.gas_limit # local_cache_attribute data_total = msg.data_length # local_cache_attribute @@ -57,11 +56,8 @@ async def sign_tx_eip1559( await paths.validate_path(keychain, msg.address_n) - # Handle ERC20s - token, address_bytes, recipient, value = await handle_erc20(msg, defs) - - if token is None and data_total > 0: - await require_confirm_data(msg.data_initial_chunk, data_total) + address_bytes = bytes_from_address(msg.to) + token, value = await sign_tx_common(msg, defs, address_bytes) await require_confirm_tx_eip1559( recipient, @@ -74,6 +70,7 @@ async def sign_tx_eip1559( bool(msg.chunkify), ) + data_total = msg.data_length data = bytearray() data += msg.data_initial_chunk data_left = data_total - len(msg.data_initial_chunk) diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index fa4ccb2aa..4e2c092c5 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -903,7 +903,7 @@ async def confirm_properties( from ubinascii import hexlify def handle_bytes(prop: PropertyType): - if isinstance(prop[1], bytes): + if isinstance(prop[1], (bytes, bytearray, memoryview)): return (prop[0], hexlify(prop[1]).decode(), True) else: # When there is not space in the text, taking it as data diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index 4f5f0d6fc..d36366db0 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -890,7 +890,10 @@ async def confirm_properties( br_code: ButtonRequestType = ButtonRequestType.ConfirmOutput, ) -> None: # Monospace flag for values that are bytes. - items = [(prop[0], prop[1], isinstance(prop[1], bytes)) for prop in props] + items = [ + (prop[0], prop[1], isinstance(prop[1], (bytes, bytearray, memoryview))) + for prop in props + ] await raise_if_not_confirmed( interact( diff --git a/tests/device_tests/ethereum/test_signtx.py b/tests/device_tests/ethereum/test_signtx.py index 465bfb82b..0cfa24d99 100644 --- a/tests/device_tests/ethereum/test_signtx.py +++ b/tests/device_tests/ethereum/test_signtx.py @@ -158,6 +158,139 @@ def test_signtx_eip1559(client: Client, chunkify: bool, parameters: dict, result assert sig_v == result["sig_v"] +def test_signtx_erc20_balanceof_misaligned_data(client: Client): + # "Data" field for a 'balanceOf' call with two last bytes removed. + with pytest.raises(TrezorFailure): + with client: + ethereum.sign_tx_eip1559( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0, + gas_limit=20, + max_gas_fee=20, + max_priority_fee=1, + to=TO_ADDR, + chain_id=1, + value=0, + data=bytes.fromhex( + "70a08231000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea009188" + ), + ) + + +def test_signtx_erc20_balanceof(client: Client): + # "Data" field for a 'balanceOf' call. The function checks the balance of the address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b + # The function has 1 argument: 'address' + with client: + ethereum.sign_tx_eip1559( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0, + gas_limit=20, + max_gas_fee=20, + max_priority_fee=1, + to=TO_ADDR, + chain_id=1, + value=0, + data=bytes.fromhex( + "70a08231000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b" + ), + ) + + +def test_signtx_erc20_allowance(client: Client): + # "Data" field for an 'allowance' call. This function checks the amount of tokens that an owner allowed to a spender. + # The function has 2 arguments: 'address', 'address' + with client: + ethereum.sign_tx_eip1559( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0, + gas_limit=20, + max_gas_fee=20, + max_priority_fee=1, + to=TO_ADDR, + chain_id=1, + value=0, + data=bytes.fromhex( + "dd62ed3e0000000000000000000000001111111111111111111111111111111111111111000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b" + ), + ) + + +def test_signtx_erc20_transferfrom(client: Client): + # "Data" field for a 'transferFrom' call. The function allows a spender to transfer a certain amount of tokens from the owner's balance to another address. + # The function has 3 arguments: 'from', 'to', 'value' + with client: + ethereum.sign_tx_eip1559( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0, + gas_limit=20, + max_gas_fee=20, + max_priority_fee=1, + to=TO_ADDR, + chain_id=1, + value=0, + data=bytes.fromhex( + "23b872dd0000000000000000000000001111111111111111111111111111111111111111000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000064" + ), + ) + + +def test_signtx_erc20_invity(client: Client): + # https://eth1.trezor.io/tx/0xcce715f7bd2fb509a41f64519d29e5d666e5f2f664ea18ce486d446e83466e56 + # "nonce": "0x10", + # "gasPrice": "0x737be7600", + # "gas": "0x39805", + # "to": "0x1111111254EEB25477B68fb85Ed929f73A960582", + # "value": "0x26b81237cb570000", + # "input": "0x12aa3caf00000000000000000000000092f3f71cef740ed5784874b8c70ff87ecdf33588000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000092f3f71cef740ed5784874b8c70ff87ecdf335880000000000000000000000007490ca03e52adbd7a05ed8cc30a228aeb968ae2500000000000000000000000000000000000000000000000026b81237cb5700000000000000000000000000000000000000000000000000000000000128cd0eef000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000160000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e10000000000000000000000000000000000c300009500004600002c000026e021973ee919693206e122950ff4b162ac52eb6248db00000000000000000080db5f7200e00000206b4be0b94041c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2d0e30db002a00000000000000000000000000000000000000000000000000000000128cd0eefee63c1e50088e6a0c2ddd26feeb64f039a2c41296fcb3f5640c02aaa39b223fe8d0a0e5c4f27ead9083c756cc280a06c4eca27a0b86991c6218b36c1d19d4a2e9eb0ce3606eb481111111254eeb25477b68fb85ed929f73a96058200000000000000000000000000000000000000000000000000000000000000cb1c12d1", + # "hash": "0xcce715f7bd2fb509a41f64519d29e5d666e5f2f664ea18ce486d446e83466e56", + # "blockNumber": "0x10fed3b", + # "from": "0x7490CA03E52ADbD7A05ed8cc30a228AeB968ae25", + # "transactionIndex": "0x72" + with client: + ethereum.sign_tx( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0x10, + gas_price=0x737BE7600, + gas_limit=0x39805, + to="0x1111111254EEB25477B68fb85Ed929f73A960582", + chain_id=1, + value=0x26B81237CB570000, # TODO this wouldn't pass the check for smart contract + tx_type=None, + data=bytes.fromhex( + "12aa3caf00000000000000000000000092f3f71cef740ed5784874b8c70ff87ecdf33588000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000092f3f71cef740ed5784874b8c70ff87ecdf335880000000000000000000000007490ca03e52adbd7a05ed8cc30a228aeb968ae2500000000000000000000000000000000000000000000000026b81237cb5700000000000000000000000000000000000000000000000000000000000128cd0eef000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000160000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e10000000000000000000000000000000000c300009500004600002c000026e021973ee919693206e122950ff4b162ac52eb6248db00000000000000000080db5f7200e00000206b4be0b94041c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2d0e30db002a00000000000000000000000000000000000000000000000000000000128cd0eefee63c1e50088e6a0c2ddd26feeb64f039a2c41296fcb3f5640c02aaa39b223fe8d0a0e5c4f27ead9083c756cc280a06c4eca27a0b86991c6218b36c1d19d4a2e9eb0ce3606eb481111111254eeb25477b68fb85ed929f73a96058200000000000000000000000000000000000000000000000000000000000000cb1c12d1" + ), + ) + + +def test_signtx_erc20_args_test(client: Client): + # Testing various data types in ERC-20 function arguments + arg0_int = "0000000000000000000000000000000000000000000000000000000000001251" + arg1_str = "00000000000000000000000000000000000000000000000000000048656c6c6f" + arg2_bytes = "1111111122222222333333334444444455555555666666667777777788888888" + arg3_address = "000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b" + + with client: + ethereum.sign_tx_eip1559( + client, + n=parse_path("m/44h/60h/0h/0/0"), + nonce=0, + gas_limit=20, + max_gas_fee=20, + max_priority_fee=1, + to=TO_ADDR, + chain_id=1, + value=0, + data=bytes.fromhex( + "00000042" + arg0_int + arg1_str + arg2_bytes + arg3_address + ), + ) + + def test_sanity_checks(client: Client): """Is not vectorized because these are internal-only tests that do not need to be exposed to the public.