diff --git a/legacy/firmware/fsm_msg_coin.h b/legacy/firmware/fsm_msg_coin.h index bca2a88a8..692671cf5 100644 --- a/legacy/firmware/fsm_msg_coin.h +++ b/legacy/firmware/fsm_msg_coin.h @@ -35,6 +35,11 @@ void fsm_msgGetPublicKey(const GetPublicKey *msg) { curve = msg->ecdsa_curve_name; } + // XXX note to future developers: + // If more path restrictions are added here, don't forget to also check + // EthereumGetPublicKey in particular for whether it's possible to go around + // the new restrictions that way. + // UnlockPath is required to access SLIP25 paths. if (msg->address_n_count > 0 && msg->address_n[0] == PATH_SLIP25_PURPOSE) { // Verify that the desired path lies in the unlocked subtree. diff --git a/legacy/firmware/fsm_msg_ethereum.h b/legacy/firmware/fsm_msg_ethereum.h index 7e2f5c8d4..bbb26a533 100644 --- a/legacy/firmware/fsm_msg_ethereum.h +++ b/legacy/firmware/fsm_msg_ethereum.h @@ -75,6 +75,17 @@ void fsm_msgEthereumGetPublicKey(const EthereumGetPublicKey *msg) { const CoinInfo *coin = fsm_getCoin(true, "Bitcoin"); if (!coin) return; + // Only allow m/44' and m/45' subtrees. This allows usage with _any_ SLIP-44 + // (Ethereum or otherwise), plus the Casa multisig subtree. Anything else must + // go through (a) GetPublicKey or (b) a dedicated coin-specific message. + if (!msg->address_n_count || (msg->address_n[0] != (44 | PATH_HARDENED) && + msg->address_n[0] != (45 | PATH_HARDENED))) { + fsm_sendFailure(FailureType_Failure_DataError, + _("Invalid path for EthereumGetPublicKey")); + layoutHome(); + return; + } + const char *curve = coin->curve_name; uint32_t fingerprint; HDNode *node = fsm_getDerivedNode(curve, msg->address_n, msg->address_n_count, diff --git a/tests/device_tests/ethereum/test_getpublickey.py b/tests/device_tests/ethereum/test_getpublickey.py index c9d1f3ed5..711ca2946 100644 --- a/tests/device_tests/ethereum/test_getpublickey.py +++ b/tests/device_tests/ethereum/test_getpublickey.py @@ -18,6 +18,7 @@ import pytest from trezorlib import ethereum from trezorlib.debuglink import TrezorClientDebugLink as Client +from trezorlib.exceptions import TrezorFailure from trezorlib.tools import parse_path from ...common import parametrize_using_common_fixtures @@ -35,3 +36,16 @@ def test_ethereum_getpublickey(client: Client, parameters, result): assert res.node.chain_code.hex() == result["chain_code"] assert res.node.public_key.hex() == result["public_key"] assert res.xpub == result["xpub"] + + +def test_slip25_disallowed(client: Client): + path = parse_path("m/10025'/60'/0'/0/0") + with pytest.raises(TrezorFailure): + ethereum.get_public_node(client, path) + + +@pytest.mark.skip_t2 +def test_legacy_restrictions(client: Client): + path = parse_path("m/46'") + with pytest.raises(TrezorFailure, match="Invalid path for EthereumGetPublicKey"): + ethereum.get_public_node(client, path) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 7a83959b3..f372a324b 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -408,6 +408,8 @@ "T1_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters0-result0]": "37e446e17465ce9823c34c9162d94e4ad3c84faa86bae7966c753be5c5fd77a2", "T1_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters1-result1]": "37e446e17465ce9823c34c9162d94e4ad3c84faa86bae7966c753be5c5fd77a2", "T1_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters2-result2]": "37e446e17465ce9823c34c9162d94e4ad3c84faa86bae7966c753be5c5fd77a2", +"T1_ethereum-test_getpublickey.py::test_legacy_restrictions": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", +"T1_ethereum-test_getpublickey.py::test_slip25_disallowed": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_blind[array_of_structs]": "04b94ca842aa77fdc6b75b07bbe178c4ea99c35e6dae630588fa5d9cb67b9090", "T1_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_blind[bare_minimum]": "730a5d3bf1389ac14aec7ee9f65e2cbf516957e5077acc0e07a7541f667e2f0b", "T1_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data_blind[basic_data]": "fd99dc685a4a125db0f3f8d2a16ffa242bd57e4ff714d66cc9c4161c486f53be", @@ -1481,6 +1483,7 @@ "TT_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters0-result0]": "66d489ec9fc951219d16eb4ed51f966510df137eabba5df68a3bd50b42cc1d1a", "TT_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters1-result1]": "66d489ec9fc951219d16eb4ed51f966510df137eabba5df68a3bd50b42cc1d1a", "TT_ethereum-test_getpublickey.py::test_ethereum_getpublickey[parameters2-result2]": "66d489ec9fc951219d16eb4ed51f966510df137eabba5df68a3bd50b42cc1d1a", +"TT_ethereum-test_getpublickey.py::test_slip25_disallowed": "80a6e289138a604cf351a29511cf6f85e2243591317894703152787e1351a1a3", "TT_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[array_of_structs]": "d8915973909efac6657a87d8369bffe91cb280ff8fcec0bec79a63a8256ebc34", "TT_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[bare_minimum]": "91fd7f2a1d5fdfdc45a9060930178ce749c11f62c079b13da6e38c154267fa67", "TT_ethereum-test_sign_typed_data.py::test_ethereum_sign_typed_data[basic_data]": "779ed1029d6810bca189456c46fa35ff2a958aba0207b4f49738d4b2fa8f9414",