diff --git a/core/.changelog.d/noissue.security b/core/.changelog.d/noissue.security new file mode 100644 index 000000000..471cda364 --- /dev/null +++ b/core/.changelog.d/noissue.security @@ -0,0 +1 @@ +Disable all testnet coins from accessing Bitcoin paths. diff --git a/core/.changelog.d/noissue.security.1 b/core/.changelog.d/noissue.security.1 new file mode 100644 index 000000000..87d744983 --- /dev/null +++ b/core/.changelog.d/noissue.security.1 @@ -0,0 +1 @@ +Restrict BIP-45 paths to Bitcoin and coins with strong replay protection. diff --git a/core/src/apps/bitcoin/keychain.py b/core/src/apps/bitcoin/keychain.py index 75ca1071c..13948e651 100644 --- a/core/src/apps/bitcoin/keychain.py +++ b/core/src/apps/bitcoin/keychain.py @@ -1,4 +1,5 @@ import gc +from micropython import const from trezor import wire from trezor.enums import InputScriptType @@ -61,6 +62,12 @@ PATTERN_UNCHAINED_UNHARDENED = ( ) PATTERN_UNCHAINED_DEPRECATED = "m/45'/coin_type'/account'/[0-1000000]/address_index" +# SLIP-44 coin type for Bitcoin +SLIP44_BITCOIN = const(0) + +# SLIP-44 coin type for all Testnet coins +SLIP44_TESTNET = const(1) + def validate_path_against_script_type( coin: coininfo.CoinInfo, @@ -82,7 +89,7 @@ def validate_path_against_script_type( if script_type == InputScriptType.SPENDADDRESS and not multisig: patterns.append(PATTERN_BIP44) - if coin.coin_name in BITCOIN_NAMES: + if coin.slip44 == SLIP44_BITCOIN: patterns.append(PATTERN_GREENADDRESS_A) patterns.append(PATTERN_GREENADDRESS_B) @@ -90,11 +97,15 @@ def validate_path_against_script_type( script_type in (InputScriptType.SPENDADDRESS, InputScriptType.SPENDMULTISIG) and multisig ): - patterns.append(PATTERN_BIP45) patterns.append(PATTERN_PURPOSE48_RAW) - if coin.coin_name in BITCOIN_NAMES: + if coin.slip44 == SLIP44_BITCOIN or ( + coin.fork_id is not None and coin.slip44 != SLIP44_TESTNET + ): + patterns.append(PATTERN_BIP45) + if coin.slip44 == SLIP44_BITCOIN: patterns.append(PATTERN_GREENADDRESS_A) patterns.append(PATTERN_GREENADDRESS_B) + if coin.coin_name in BITCOIN_NAMES: patterns.append(PATTERN_UNCHAINED_HARDENED) patterns.append(PATTERN_UNCHAINED_UNHARDENED) patterns.append(PATTERN_UNCHAINED_DEPRECATED) @@ -103,16 +114,17 @@ def validate_path_against_script_type( patterns.append(PATTERN_BIP49) if multisig: patterns.append(PATTERN_PURPOSE48_P2SHSEGWIT) - if coin.coin_name in BITCOIN_NAMES: + if coin.slip44 == SLIP44_BITCOIN: patterns.append(PATTERN_GREENADDRESS_A) patterns.append(PATTERN_GREENADDRESS_B) + if coin.coin_name in BITCOIN_NAMES: patterns.append(PATTERN_CASA) elif coin.segwit and script_type == InputScriptType.SPENDWITNESS: patterns.append(PATTERN_BIP84) if multisig: patterns.append(PATTERN_PURPOSE48_SEGWIT) - if coin.coin_name in BITCOIN_NAMES: + if coin.slip44 == SLIP44_BITCOIN: patterns.append(PATTERN_GREENADDRESS_A) patterns.append(PATTERN_GREENADDRESS_B) @@ -125,18 +137,29 @@ def get_schemas_for_coin(coin: coininfo.CoinInfo) -> Iterable[PathSchema]: # basic patterns patterns = [ PATTERN_BIP44, - PATTERN_BIP45, PATTERN_PURPOSE48_RAW, ] - # compatibility patterns - if coin.coin_name in BITCOIN_NAMES: + # patterns without coin_type field must be treated as if coin_type == 0 + if coin.slip44 == SLIP44_BITCOIN or ( + coin.fork_id is not None and coin.slip44 != SLIP44_TESTNET + ): + patterns.append(PATTERN_BIP45) + + if coin.slip44 == SLIP44_BITCOIN: patterns.extend( ( PATTERN_GREENADDRESS_A, PATTERN_GREENADDRESS_B, PATTERN_GREENADDRESS_SIGN_A, PATTERN_GREENADDRESS_SIGN_B, + ) + ) + + # compatibility patterns + if coin.coin_name in BITCOIN_NAMES: + patterns.extend( + ( PATTERN_CASA, PATTERN_UNCHAINED_HARDENED, PATTERN_UNCHAINED_UNHARDENED, @@ -157,11 +180,16 @@ def get_schemas_for_coin(coin: coininfo.CoinInfo) -> Iterable[PathSchema]: schemas = [PathSchema.parse(pattern, coin.slip44) for pattern in patterns] - # some wallets such as Electron-Cash (BCH) store coins on Bitcoin paths - # we can allow spending these coins from Bitcoin paths if the coin has - # implemented strong replay protection via SIGHASH_FORKID - if coin.fork_id is not None: - schemas.extend(PathSchema.parse(pattern, 0) for pattern in patterns) + # Some wallets such as Electron-Cash (BCH) store coins on Bitcoin paths. + # We can allow spending these coins from Bitcoin paths if the coin has + # implemented strong replay protection via SIGHASH_FORKID. However, we + # cannot allow spending any testnet coins from Bitcoin paths, because + # otherwise an attacker could trick the user into spending BCH on a Bitcoin + # path by signing a seemingly harmless BCH Testnet transaction. + if coin.fork_id is not None and coin.slip44 != SLIP44_TESTNET: + schemas.extend( + PathSchema.parse(pattern, SLIP44_BITCOIN) for pattern in patterns + ) gc.collect() return [schema.copy() for schema in schemas] diff --git a/core/tests/test_apps.bitcoin.keychain.py b/core/tests/test_apps.bitcoin.keychain.py index 611c73fe3..695a272e9 100644 --- a/core/tests/test_apps.bitcoin.keychain.py +++ b/core/tests/test_apps.bitcoin.keychain.py @@ -53,15 +53,11 @@ class TestBitcoinKeychain(unittest.TestCase): valid_addresses = ( [H_(44), H_(1), H_(0), 0, 0], - [H_(45), 99, 1, 1000], [H_(48), H_(1), H_(0), H_(2), 1, 1000], [H_(49), H_(1), H_(0), 0, 10], [H_(84), H_(1), H_(0), 0, 10], # Casa: [49, 1, 0, 0, 10], - # Green: - [1, 1000], - [H_(3), H_(10), 4, 1000], ) invalid_addresses = ( [H_(43), H_(1), H_(0), 0, 0], @@ -69,6 +65,10 @@ class TestBitcoinKeychain(unittest.TestCase): [44, 1, 0, 0, 0], [H_(44), H_(1), H_(0)], [H_(44), H_(1), H_(0), 0, 0, 0], + [H_(45), 99, 1, 1000], + # Green: + [1, 1000], + [H_(3), H_(10), 4, 1000], ) for addr in valid_addresses: @@ -141,7 +141,6 @@ class TestAltcoinKeychains(unittest.TestCase): self.assertTrue(coin.segwit) valid_addresses = ( [H_(44), H_(2), H_(0), 0, 0], - [H_(45), 99, 1, 1000], [H_(48), H_(2), H_(0), H_(2), 1, 1000], [H_(49), H_(2), H_(0), 0, 10], [H_(84), H_(2), H_(0), 0, 10], @@ -150,6 +149,7 @@ class TestAltcoinKeychains(unittest.TestCase): [H_(43), H_(2), H_(0), 0, 0], # Bitcoin paths: [H_(44), H_(0), H_(0), 0, 0], + [H_(45), 99, 1, 1000], [H_(49), H_(0), H_(0), 0, 0], [H_(84), H_(0), H_(0), 0, 0],