diff --git a/legacy/firmware/.changelog.d/1660.fixed b/legacy/firmware/.changelog.d/1660.fixed new file mode 100644 index 0000000000..badda7e786 --- /dev/null +++ b/legacy/firmware/.changelog.d/1660.fixed @@ -0,0 +1 @@ +Allow non-standard paths used by Unchained Capital, Green Address and Casa. diff --git a/legacy/firmware/.changelog.d/noissue.security.4 b/legacy/firmware/.changelog.d/noissue.security.4 new file mode 100644 index 0000000000..87d7449830 --- /dev/null +++ b/legacy/firmware/.changelog.d/noissue.security.4 @@ -0,0 +1 @@ +Restrict BIP-45 paths to Bitcoin and coins with strong replay protection. diff --git a/legacy/firmware/crypto.c b/legacy/firmware/crypto.c index 1ed5f21c30..da07aa44c3 100644 --- a/legacy/firmware/crypto.c +++ b/legacy/firmware/crypto.c @@ -555,15 +555,49 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, return valid; } - // m/45' - BIP45 Copay Abandoned Multisig P2SH - // m / purpose' / cosigner_index / change / address_index if (address_n_count > 0 && address_n[0] == (0x80000000 + 45)) { + if (address_n_count == 4) { + // m/45' - BIP45 Copay Abandoned Multisig P2SH + // m / purpose' / cosigner_index / change / address_index + // Patterns without a coin_type field must be treated as Bitcoin paths. + valid &= check_cointype(coin, SLIP44_BITCOIN, full); + } else if (address_n_count == 5) { + // Unchained Capital compatibility pattern. Will be removed in the + // future. + // m / 45' / coin_type' / account' / [0-1000000] / address_index + valid &= check_cointype(coin, address_n[1], full); + } else if (address_n_count == 6) { + // Unchained Capital compatibility pattern. Will be removed in the + // future. + // m/45'/coin_type'/account'/[0-1000000]/change/address_index + // m/45'/coin_type/account/[0-1000000]/change/address_index + valid &= check_cointype(coin, 0x80000000 | address_n[1], full); + } + if (full) { - valid &= (script_type == InputScriptType_SPENDMULTISIG); - valid &= (address_n_count == 4); - valid &= (address_n[1] & 0x80000000) == 0; - valid &= (address_n[2] & 0x80000000) == 0; - valid &= (address_n[3] & 0x80000000) == 0; + if (address_n_count == 4) { + valid &= (script_type == InputScriptType_SPENDMULTISIG); + valid &= (address_n[1] & 0x80000000) == 0; + valid &= (address_n[2] & 0x80000000) == 0; + valid &= (address_n[3] & 0x80000000) == 0; + } else if (address_n_count == 5) { + valid &= (script_type == InputScriptType_SPENDADDRESS || + script_type == InputScriptType_SPENDMULTISIG); + valid &= (address_n[2] & 0x80000000) == 0x80000000; + valid &= (address_n[2] & 0x7fffffff) <= 100; + valid &= address_n[3] <= 1000000; + valid &= address_n[4] <= 1000000; + } else if (address_n_count == 6) { + valid &= (script_type == InputScriptType_SPENDADDRESS || + script_type == InputScriptType_SPENDMULTISIG); + valid &= (address_n[1] & 0x80000000) == (address_n[2] & 0x80000000); + valid &= (address_n[2] & 0x7fffffff) <= 100; + valid &= address_n[3] <= 1000000; + valid &= address_n[4] <= 1; + valid &= address_n[5] <= 1000000; + } else { + return false; + } } return valid; } @@ -628,6 +662,66 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, return valid; } + // Green Address compatibility pattern. Will be removed in the future. + // m / [1,4] / address_index + if (address_n_count > 0 && (address_n[0] == 1 || address_n[0] == 4)) { + valid &= (coin->coin_type == SLIP44_BITCOIN); + if (full) { + valid &= (address_n_count == 2); + valid &= (address_n[1] <= 1000000); + } + return valid; + } + + // Green Address compatibility pattern. Will be removed in the future. + // m / 3' / [1-100]' / [1,4] / address_index + if (address_n_count > 0 && address_n[0] == (0x80000000 + 3)) { + valid &= (coin->coin_type == SLIP44_BITCOIN); + if (full) { + valid &= (address_n_count == 4); + valid &= (address_n[1] & 0x80000000) == 0x80000000; + valid &= (address_n[1] & 0x7fffffff) <= 100; + valid &= address_n[2] == 1 || address_n[2] == 4; + valid &= address_n[3] <= 1000000; + } + return valid; + } + + // Green Address compatibility patterns. Will be removed in the future. + // m / 1195487518 + // m / 1195487518 / 6 / address_index + if (address_n_count > 0 && address_n[0] == 1195487518) { + valid &= (coin->coin_type == SLIP44_BITCOIN); + if (full) { + if (address_n_count == 3) { + valid &= (address_n[1] == 6); + valid &= (address_n[2] <= 1000000); + } else if (address_n_count != 1) { + return false; + } + } + return valid; + } + + // Casa compatibility pattern. Will be removed in the future. + // m / 49 / coin_type / account / change / address_index + if (address_n_count > 0 && address_n[0] == 49) { + if (full) { + valid &= (address_n_count == 5); + } else { + valid &= (address_n_count >= 2); + } + valid &= check_cointype(coin, 0x80000000 | address_n[1], full); + if (full) { + valid &= (script_type == InputScriptType_SPENDP2SHWITNESS); + valid &= (address_n[1] & 0x80000000) == 0; + valid &= address_n[2] <= 100; + valid &= address_n[3] <= 1; + valid &= address_n[4] <= 1000000; + } + return valid; + } + // we don't check unknown paths return true; } diff --git a/tests/device_tests/test_nonstandard_paths.py b/tests/device_tests/test_nonstandard_paths.py index 7a8d849678..8082779b45 100644 --- a/tests/device_tests/test_nonstandard_paths.py +++ b/tests/device_tests/test_nonstandard_paths.py @@ -109,12 +109,6 @@ def test_getpublicnode(client, path, script_types): assert res.xpub -# See https://github.com/trezor/trezor-firmware/issues/1660 -# T1 fails on: -# test_getaddress[m/45'/0'/63'/1000000/0/255-script_types5] -# test_getaddress[m/45'/0'/63'/1000000/255-script_types7] -# test_getaddress[m/45'/0/63/1000000/0/255-script_types6] -@pytest.mark.skip_t1 @pytest.mark.parametrize("path, script_types", VECTORS) def test_getaddress(client, path, script_types): for script_type in script_types: @@ -170,9 +164,6 @@ def test_signtx(client, path, script_types): assert serialized_tx.hex() -# See https://github.com/trezor/trezor-firmware/issues/1660 -# T1 fails on the Unchained paths -@pytest.mark.skip_t1 @pytest.mark.multisig @pytest.mark.parametrize("paths, address_index", VECTORS_MULTISIG) def test_getaddress_multisig(client, paths, address_index):