diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py index 05f78a0b1..50f108d68 100644 --- a/core/src/apps/ethereum/sign_tx.py +++ b/core/src/apps/ethereum/sign_tx.py @@ -177,14 +177,13 @@ def check(msg: EthereumSignTx) -> None: if msg.tx_type not in [1, 6, None]: raise wire.DataError("tx_type out of bounds") - check_data(msg) + if len(msg.gas_price) + len(msg.gas_limit) > 30: + raise wire.DataError("Fee overflow") - # safety checks - if not check_gas(msg) or not check_to(msg): - raise wire.DataError("Safety check failed") + check_common_fields(msg) -def check_data(msg: EthereumSignTxAny) -> None: +def check_common_fields(msg: EthereumSignTxAny) -> None: if msg.data_length > 0: if not msg.data_initial_chunk: raise wire.DataError("Data length provided, but no initial chunk") @@ -195,20 +194,12 @@ def check_data(msg: EthereumSignTxAny) -> None: if len(msg.data_initial_chunk) > msg.data_length: raise wire.DataError("Invalid size of initial chunk") + if len(msg.to) not in (0, 40, 42): + raise wire.DataError("Invalid recipient address") -def check_gas(msg: EthereumSignTx) -> bool: - if len(msg.gas_price) + len(msg.gas_limit) > 30: - # sanity check that fee doesn't overflow - return False - return True - + if not msg.to and msg.data_length == 0: + # sending transaction to address 0 (contract creation) without a data field + raise wire.DataError("Contract creation without data") -def check_to(msg: EthereumSignTxAny) -> bool: - if msg.to == "": - if msg.data_length == 0: - # sending transaction to address 0 (contract creation) without a data field - return False - else: - if len(msg.to) not in (40, 42): - return False - return True + if msg.chain_id == 0: + raise wire.DataError("Chain ID out of bounds") diff --git a/core/src/apps/ethereum/sign_tx_eip1559.py b/core/src/apps/ethereum/sign_tx_eip1559.py index f08437360..9d06f1ee7 100644 --- a/core/src/apps/ethereum/sign_tx_eip1559.py +++ b/core/src/apps/ethereum/sign_tx_eip1559.py @@ -14,7 +14,7 @@ from .layout import ( require_confirm_eip1559_fee, require_confirm_tx, ) -from .sign_tx import check_data, check_to, handle_erc20, send_request_chunk +from .sign_tx import check_common_fields, handle_erc20, send_request_chunk if False: from typing import Tuple @@ -159,7 +159,9 @@ def sign_digest( def check(msg: EthereumSignTxEIP1559) -> None: - check_data(msg) + if len(msg.max_gas_fee) + len(msg.gas_limit) > 30: + raise wire.DataError("Fee overflow") + if len(msg.max_priority_fee) + len(msg.gas_limit) > 30: + raise wire.DataError("Fee overflow") - if not check_to(msg): - raise wire.DataError("Safety check failed") + check_common_fields(msg) diff --git a/tests/device_tests/ethereum/test_signtx.py b/tests/device_tests/ethereum/test_signtx.py index 9b6f089ea..4ec4965e7 100644 --- a/tests/device_tests/ethereum/test_signtx.py +++ b/tests/device_tests/ethereum/test_signtx.py @@ -83,7 +83,7 @@ def test_sanity_checks(client): need to be exposed to the public. """ # contract creation without data should fail. - with pytest.raises(Exception): + with pytest.raises(TrezorFailure, match=r"DataError"): ethereum.sign_tx( client, n=parse_path("44'/60'/0'/0/0"), @@ -92,10 +92,11 @@ def test_sanity_checks(client): gas_limit=20000, to="", value=12345678901234567890, + chain_id=1, ) # gas overflow - with pytest.raises(TrezorFailure): + with pytest.raises(TrezorFailure, match=r"DataError"): ethereum.sign_tx( client, n=parse_path("44'/60'/0'/0/0"), @@ -104,6 +105,20 @@ def test_sanity_checks(client): gas_limit=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, to=TO_ADDR, value=12345678901234567890, + chain_id=1, + ) + + # bad chain ID + with pytest.raises(TrezorFailure, match=r"Chain ID out of bounds"): + ethereum.sign_tx( + client, + n=parse_path("44'/60'/0'/0/0"), + nonce=123456, + gas_price=20000, + gas_limit=20000, + to=TO_ADDR, + value=12345678901234567890, + chain_id=0, ) @@ -250,3 +265,65 @@ def test_signtx_eip1559_access_list_larger(client): sig_s.hex() == "7d0ea2a28ef5702ca763c1f340427c0020292ffcbb4553dd1c8ea8e2b9126dbc" ) + + +@pytest.mark.skip_t1 +def test_sanity_checks_eip1559(client): + """Is not vectorized because these are internal-only tests that do not + need to be exposed to the public. + """ + # contract creation without data should fail. + with pytest.raises(TrezorFailure, match=r"DataError"): + ethereum.sign_tx_eip1559( + client, + n=parse_path("44'/60'/0'/0/100"), + nonce=0, + gas_limit=20, + to="", + chain_id=1, + value=10, + max_gas_fee=20, + max_priority_fee=1, + ) + + # max fee overflow + with pytest.raises(TrezorFailure, match=r"DataError"): + ethereum.sign_tx_eip1559( + client, + n=parse_path("44'/60'/0'/0/100"), + nonce=0, + gas_limit=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, + to=TO_ADDR, + chain_id=1, + value=10, + max_gas_fee=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, + max_priority_fee=1, + ) + + # priority fee overflow + with pytest.raises(TrezorFailure, match=r"DataError"): + ethereum.sign_tx_eip1559( + client, + n=parse_path("44'/60'/0'/0/100"), + nonce=0, + gas_limit=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, + to=TO_ADDR, + chain_id=1, + value=10, + max_gas_fee=20, + max_priority_fee=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, + ) + + # bad chain ID + with pytest.raises(TrezorFailure, match=r"Chain ID out of bounds"): + ethereum.sign_tx_eip1559( + client, + n=parse_path("44'/60'/0'/0/100"), + nonce=0, + gas_limit=20, + to=TO_ADDR, + chain_id=0, + value=10, + max_gas_fee=20, + max_priority_fee=1, + ) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index a5b03bed4..ff5a2366d 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -143,6 +143,7 @@ "ethereum-test_sign_verify_message.py::test_verify_invalid": "95241229201fd860604af3e061eb1173ebd9078e3db8679ecd72db86c7c08f63", "ethereum-test_signtx.py::test_data_streaming": "ca1a891c07f4a0bdce9fd2bd9ab519f6c6ce07806ea59d08416736605682f8f1", "ethereum-test_signtx.py::test_sanity_checks": "5a80508a71a9ef64f94762b07636f90e464832f0f4a3102af8fa1a8c69e94586", +"ethereum-test_signtx.py::test_sanity_checks_eip1559": "5a80508a71a9ef64f94762b07636f90e464832f0f4a3102af8fa1a8c69e94586", "ethereum-test_signtx.py::test_signtx[Auxilium]": "723083f8dccf4676f7a134a070977576341f2f7a5850e463e487c8705ebe25f1", "ethereum-test_signtx.py::test_signtx[ETC]": "41ed0476f3043025d40eaf13ed76c1ebcd4bee9b7a6d23764a00451f1edc5b40", "ethereum-test_signtx.py::test_signtx[Ethereum]": "ad88572a4100efd3909c39c76e452630f25d5600bcde22a18c02c327528507f3",