From 5fb0f9a6c58b77ff9ed5c2c665658ad8fc3327ac Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Tue, 29 Jun 2021 13:38:59 +0200 Subject: [PATCH] feat(legacy): Distinguish between known path checks and script type checks in coin_path_check(). --- legacy/firmware/crypto.c | 124 ++++++++++++++++++++------------- legacy/firmware/crypto.h | 12 +++- legacy/firmware/fsm_msg_coin.h | 10 +-- legacy/firmware/signing.c | 5 +- 4 files changed, 94 insertions(+), 57 deletions(-) diff --git a/legacy/firmware/crypto.c b/legacy/firmware/crypto.c index d3b138135a..7b3d498359 100644 --- a/legacy/firmware/crypto.c +++ b/legacy/firmware/crypto.c @@ -537,30 +537,37 @@ static bool check_cointype(const CoinInfo *coin, uint32_t slip44, bool full) { return coin->coin_type == slip44; } -bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, - uint32_t address_n_count, const uint32_t *address_n, - bool has_multisig, bool full) { - // If full == true, this function checks that the path is a recognized path - // for the given coin. Used by GetAddress to prevent ransom attacks where a - // user could be coerced to use an address with an unenumerable path. - // If full == false, this function checks that a coin without strong replay +bool coin_path_check(const CoinInfo *coin, InputScriptType script_type, + uint32_t address_n_count, const uint32_t *address_n, + bool has_multisig, CoinPathCheckLevel level) { + // For level BASIC this function checks that a coin without strong replay // protection doesn't access paths that are known to be used by another coin. // Used by SignTx to ensure that a user cannot be coerced into signing a // testnet transaction or a Litecoin transaction which in fact spends Bitcoin. + // For level KNOWN this function checks that the path is a recognized path for + // the given coin. Used by GetAddress to prevent ransom attacks where a user + // could be coerced to use an address with an unenumerable path. + // For level SCRIPT_TYPE this function makes the same checks as in level + // KNOWN, but includes script type checks. + + const bool check_known = (level >= CoinPathCheckLevel_KNOWN); + const bool check_script_type = (level >= CoinPathCheckLevel_SCRIPT_TYPE); bool valid = true; // m/44' : BIP44 Legacy // m / purpose' / coin_type' / account' / change / address_index if (address_n_count > 0 && address_n[0] == (0x80000000 + 44)) { - if (full) { + if (check_known) { valid = valid && (address_n_count == 5); } else { valid = valid && (address_n_count >= 2); } - valid = valid && check_cointype(coin, address_n[1], full); - if (full) { + valid = valid && check_cointype(coin, address_n[1], check_known); + if (check_script_type) { valid = valid && (script_type == InputScriptType_SPENDADDRESS); valid = valid && (!has_multisig); + } + if (check_known) { valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); valid = valid && (address_n[3] <= PATH_MAX_CHANGE); @@ -570,51 +577,63 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, } if (address_n_count > 0 && address_n[0] == (0x80000000 + 45)) { + if (check_script_type) { + valid = valid && has_multisig; + } + 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 = valid && check_cointype(coin, SLIP44_BITCOIN, full); + valid = valid && check_cointype(coin, SLIP44_BITCOIN, check_known); + if (check_script_type) { + valid = valid && (script_type == InputScriptType_SPENDMULTISIG); + } + if (check_known) { + valid = valid && (address_n[1] <= 100); + valid = valid && (address_n[2] <= PATH_MAX_CHANGE); + valid = valid && (address_n[3] <= PATH_MAX_ADDRESS_INDEX); + } } 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 = valid && check_cointype(coin, address_n[1], full); + valid = valid && check_cointype(coin, address_n[1], check_known); + if (check_script_type) { + valid = valid && (script_type == InputScriptType_SPENDADDRESS || + script_type == InputScriptType_SPENDMULTISIG); + } + if (check_known) { + valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); + valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); + valid = valid && (address_n[3] <= 1000000); + valid = valid && (address_n[4] <= PATH_MAX_ADDRESS_INDEX); + } } 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 = valid && check_cointype(coin, 0x80000000 | address_n[1], full); - } - - if (full) { - valid = valid && has_multisig; - if (address_n_count == 4) { - valid = valid && (script_type == InputScriptType_SPENDMULTISIG); - valid = valid && (address_n[1] <= 100); - valid = valid && (address_n[2] <= PATH_MAX_CHANGE); - valid = valid && (address_n[3] <= PATH_MAX_ADDRESS_INDEX); - } else if (address_n_count == 5) { - valid = valid && (script_type == InputScriptType_SPENDADDRESS || - script_type == InputScriptType_SPENDMULTISIG); - valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); - valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); - valid = valid && (address_n[3] <= 1000000); - valid = valid && (address_n[4] <= PATH_MAX_ADDRESS_INDEX); - } else if (address_n_count == 6) { + valid = + valid && check_cointype(coin, 0x80000000 | address_n[1], check_known); + if (check_script_type) { valid = valid && (script_type == InputScriptType_SPENDADDRESS || script_type == InputScriptType_SPENDMULTISIG); + } + if (check_known) { valid = valid && ((address_n[1] & 0x80000000) == (address_n[2] & 0x80000000)); valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); valid = valid && (address_n[3] <= 1000000); valid = valid && (address_n[4] <= PATH_MAX_CHANGE); valid = valid && (address_n[5] <= PATH_MAX_ADDRESS_INDEX); - } else { + } + } else { + if (check_known) { return false; } } + return valid; } @@ -623,17 +642,19 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // Electrum: // m / purpose' / coin_type' / account' / type' / change / address_index if (address_n_count > 0 && address_n[0] == (0x80000000 + 48)) { - if (full) { + if (check_known) { valid = valid && (address_n_count == 5 || address_n_count == 6); } else { valid = valid && (address_n_count >= 2); } - valid = valid && check_cointype(coin, address_n[1], full); - if (full) { + valid = valid && check_cointype(coin, address_n[1], check_known); + if (check_script_type) { valid = valid && has_multisig; valid = valid && (script_type == InputScriptType_SPENDMULTISIG || script_type == InputScriptType_SPENDP2SHWITNESS || script_type == InputScriptType_SPENDWITNESS); + } + if (check_known) { valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); if (address_n_count == 5) { @@ -655,14 +676,16 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // m / purpose' / coin_type' / account' / change / address_index if (address_n_count > 0 && address_n[0] == (0x80000000 + 49)) { valid = valid && coin->has_segwit; - if (full) { + if (check_known) { valid = valid && (address_n_count == 5); } else { valid = valid && (address_n_count >= 2); } - valid = valid && check_cointype(coin, address_n[1], full); - if (full) { + valid = valid && check_cointype(coin, address_n[1], check_known); + if (check_script_type) { valid = valid && (script_type == InputScriptType_SPENDP2SHWITNESS); + } + if (check_known) { valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); valid = valid && (address_n[3] <= PATH_MAX_CHANGE); @@ -676,14 +699,16 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, if (address_n_count > 0 && address_n[0] == (0x80000000 + 84)) { valid = valid && coin->has_segwit; valid = valid && (coin->bech32_prefix != NULL); - if (full) { + if (check_known) { valid = valid && (address_n_count == 5); } else { valid = valid && (address_n_count >= 2); } - valid = valid && check_cointype(coin, address_n[1], full); - if (full) { + valid = valid && check_cointype(coin, address_n[1], check_known); + if (check_script_type) { valid = valid && (script_type == InputScriptType_SPENDWITNESS); + } + if (check_known) { valid = valid && ((address_n[2] & 0x80000000) == 0x80000000); valid = valid && ((address_n[2] & 0x7fffffff) <= PATH_MAX_ACCOUNT); valid = valid && (address_n[3] <= PATH_MAX_CHANGE); @@ -696,7 +721,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // m / [1,4] / address_index if (address_n_count > 0 && (address_n[0] == 1 || address_n[0] == 4)) { valid = valid && (coin->coin_type == SLIP44_BITCOIN); - if (full) { + if (check_known) { valid = valid && (address_n_count == 2); valid = valid && (address_n[1] <= PATH_MAX_ADDRESS_INDEX); } @@ -707,7 +732,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // m / 3' / [1-100]' / [1,4] / address_index if (address_n_count > 0 && address_n[0] == (0x80000000 + 3)) { valid = valid && (coin->coin_type == SLIP44_BITCOIN); - if (full) { + if (check_known) { valid = valid && (address_n_count == 4); valid = valid && ((address_n[1] & 0x80000000) == 0x80000000); valid = valid && ((address_n[1] & 0x7fffffff) <= 100); @@ -722,7 +747,7 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // m / 1195487518 / 6 / address_index if (address_n_count > 0 && address_n[0] == 1195487518) { valid = valid && (coin->coin_type == SLIP44_BITCOIN); - if (full) { + if (check_known) { if (address_n_count == 3) { valid = valid && (address_n[1] == 6); valid = valid && (address_n[2] <= PATH_MAX_ADDRESS_INDEX); @@ -736,14 +761,17 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, // 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) { + if (check_known) { valid = valid && (address_n_count == 5); } else { valid = valid && (address_n_count >= 2); } - valid = valid && check_cointype(coin, 0x80000000 | address_n[1], full); - if (full) { + valid = + valid && check_cointype(coin, 0x80000000 | address_n[1], check_known); + if (check_script_type) { valid = valid && (script_type == InputScriptType_SPENDP2SHWITNESS); + } + if (check_known) { valid = valid && ((address_n[1] & 0x80000000) == 0); valid = valid && (address_n[2] <= PATH_MAX_ACCOUNT); valid = valid && (address_n[3] <= PATH_MAX_CHANGE); @@ -752,6 +780,6 @@ bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, return valid; } - // we allow unknown paths when a full check is not required - return !full; + // we allow unknown paths only when a full check is not required + return level == CoinPathCheckLevel_BASIC; } diff --git a/legacy/firmware/crypto.h b/legacy/firmware/crypto.h index c47c707f59..006db4bf24 100644 --- a/legacy/firmware/crypto.h +++ b/legacy/firmware/crypto.h @@ -32,6 +32,12 @@ #include "messages-bitcoin.pb.h" #include "messages-crypto.pb.h" +typedef enum _CoinPathCheckLevel { + CoinPathCheckLevel_BASIC = 0, + CoinPathCheckLevel_KNOWN = 1, + CoinPathCheckLevel_SCRIPT_TYPE = 2, +} CoinPathCheckLevel; + #define ser_length_size(len) ((len) < 253 ? 1 : (len) < 0x10000 ? 3 : 5) uint32_t ser_length(uint32_t len, uint8_t *out); @@ -82,8 +88,8 @@ int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig, int cryptoIdentityFingerprint(const IdentityType *identity, uint8_t *hash); -bool coin_known_path_check(const CoinInfo *coin, InputScriptType script_type, - uint32_t address_n_count, const uint32_t *address_n, - bool has_multisig, bool full); +bool coin_path_check(const CoinInfo *coin, InputScriptType script_type, + uint32_t address_n_count, const uint32_t *address_n, + bool has_multisig, CoinPathCheckLevel level); #endif diff --git a/legacy/firmware/fsm_msg_coin.h b/legacy/firmware/fsm_msg_coin.h index 6e01431423..e62bbd7ebf 100644 --- a/legacy/firmware/fsm_msg_coin.h +++ b/legacy/firmware/fsm_msg_coin.h @@ -183,11 +183,13 @@ void fsm_msgGetAddress(const GetAddress *msg) { strlcpy(desc, _("Address:"), sizeof(desc)); } - if (!coin_known_path_check(coin, msg->script_type, msg->address_n_count, - msg->address_n, msg->has_multisig, true)) { + if (!coin_path_check(coin, msg->script_type, msg->address_n_count, + msg->address_n, msg->has_multisig, + CoinPathCheckLevel_SCRIPT_TYPE)) { if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict && - !coin_known_path_check(coin, msg->script_type, msg->address_n_count, - msg->address_n, msg->has_multisig, false)) { + !coin_path_check(coin, msg->script_type, msg->address_n_count, + msg->address_n, msg->has_multisig, + CoinPathCheckLevel_KNOWN)) { fsm_sendFailure(FailureType_Failure_DataError, _("Forbidden key path")); layoutHome(); return; diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index 5cc29758c8..0e611825ec 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -685,8 +685,9 @@ bool compile_input_script_sig(TxInputType *tinput) { return false; } } - if (!coin_known_path_check(coin, tinput->script_type, tinput->address_n_count, - tinput->address_n, tinput->has_multisig, false)) { + if (!coin_path_check(coin, tinput->script_type, tinput->address_n_count, + tinput->address_n, tinput->has_multisig, + CoinPathCheckLevel_BASIC)) { if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict) { return false; }