arno/trezor-firmware
1
0
Fork 0
mirror of https://github.com/trezor/trezor-firmware.git synced 2025-01-30 17:21:21 +00:00
Code Issues Releases Wiki Activity

fix(core): Fix insufficient BIP-32 path checks.

Browse Source 
- Disable testnet coins from accessing Bitcoin paths.

- Restrict BIP-45 paths to Bitcoin and coins with strong replay protection.
This commit is contained in:
Andrew Kozlik 2021-06-23 15:41:22 +02:00 committed by Martin Milata
parent 3884232f74
commit 3e9f8a32ac
4 changed files with 48 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/.changelog.d/noissue.security Normal file
View File

@ -0,0 +1 @@
Disable all testnet coins from accessing Bitcoin paths.

1
core/.changelog.d/noissue.security.1 Normal file
View File

@ -0,0 +1 @@
Restrict BIP-45 paths to Bitcoin and coins with strong replay protection.

54
core/src/apps/bitcoin/keychain.py
View File

@ -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]

10
core/tests/test_apps.bitcoin.keychain.py
View File

@ -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],