diff --git a/legacy/firmware/.changelog.d/4396.added b/legacy/firmware/.changelog.d/4396.added new file mode 100644 index 00000000000..72b264b78ec --- /dev/null +++ b/legacy/firmware/.changelog.d/4396.added @@ -0,0 +1 @@ +Added support for lexicographic sorting of pubkeys in multisig. diff --git a/legacy/firmware/.changelog.d/4396.changed b/legacy/firmware/.changelog.d/4396.changed new file mode 100644 index 00000000000..ba68fd7fa63 --- /dev/null +++ b/legacy/firmware/.changelog.d/4396.changed @@ -0,0 +1 @@ +Forbid multisig to singlesig change outputs. diff --git a/legacy/firmware/.changelog.d/4396.changed.2 b/legacy/firmware/.changelog.d/4396.changed.2 new file mode 100644 index 00000000000..87cd7c8ae52 --- /dev/null +++ b/legacy/firmware/.changelog.d/4396.changed.2 @@ -0,0 +1 @@ +Forbid per-node paths in multisig change outputs and multisig receive addresses. diff --git a/legacy/firmware/.changelog.d/4396.changed.3 b/legacy/firmware/.changelog.d/4396.changed.3 new file mode 100644 index 00000000000..07d5ba35c8d --- /dev/null +++ b/legacy/firmware/.changelog.d/4396.changed.3 @@ -0,0 +1 @@ +Remove deprecated Unchained Capital's multisig path. diff --git a/legacy/firmware/crypto.c b/legacy/firmware/crypto.c index 08f457b1822..7cad755e27a 100644 --- a/legacy/firmware/crypto.c +++ b/legacy/firmware/crypto.c @@ -18,6 +18,7 @@ */ #include "crypto.h" +#include #include #include "address.h" #include "aes/aes.h" @@ -368,9 +369,56 @@ uint32_t cryptoMultisigPubkeyCount(const MultisigRedeemScriptType *multisig) { : multisig->pubkeys_count; } +static int comparePubkeysLexicographically(const void *first, + const void *second) { + return memcmp(first, second, 33); +} + +uint32_t cryptoMultisigPubkeys(const CoinInfo *coin, + const MultisigRedeemScriptType *multisig, + uint8_t *pubkeys) { + const uint32_t n = cryptoMultisigPubkeyCount(multisig); + if (n < 1 || n > 15) { + return 0; + } + + for (uint32_t i = 0; i < n; i++) { + const HDNode *pubnode = cryptoMultisigPubkey(coin, multisig, i); + if (!pubnode) { + return 0; + } + memcpy(pubkeys + i * 33, pubnode->public_key, 33); + } + + if (multisig->has_pubkeys_order && + multisig->pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) { + qsort(pubkeys, n, 33, comparePubkeysLexicographically); + } + + return n; +} + int cryptoMultisigPubkeyIndex(const CoinInfo *coin, const MultisigRedeemScriptType *multisig, const uint8_t *pubkey) { + uint32_t n = cryptoMultisigPubkeyCount(multisig); + + uint8_t pubkeys[33 * n]; + if (!cryptoMultisigPubkeys(coin, multisig, pubkeys)) { + return -1; + } + + for (size_t i = 0; i < n; i++) { + if (memcmp(pubkeys + i * 33, pubkey, 33) == 0) { + return i; + } + } + return -1; +} + +int cryptoMultisigXpubIndex(const CoinInfo *coin, + const MultisigRedeemScriptType *multisig, + const uint8_t *pubkey) { for (size_t i = 0; i < cryptoMultisigPubkeyCount(multisig); i++) { const HDNode *pubnode = cryptoMultisigPubkey(coin, multisig, i); if (pubnode && memcmp(pubnode->public_key, pubkey, 33) == 0) { @@ -380,9 +428,15 @@ int cryptoMultisigPubkeyIndex(const CoinInfo *coin, return -1; } +static int comparePubnodesLexicographically(const void *first, + const void *second) { + return memcmp(*(const HDNodeType **)first, *(const HDNodeType **)second, + sizeof(HDNodeType)); +} + int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig, uint8_t *hash) { - static const HDNodeType *pubnodes[15], *swap; + static const HDNodeType *pubnodes[15]; const uint32_t n = cryptoMultisigPubkeyCount(multisig); if (n < 1 || n > 15) { return 0; @@ -403,21 +457,22 @@ int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig, if (pubnodes[i]->public_key.size != 33) return 0; if (pubnodes[i]->chain_code.size != 32) return 0; } - // minsort according to pubkey - for (uint32_t i = 0; i < n - 1; i++) { - for (uint32_t j = n - 1; j > i; j--) { - if (memcmp(pubnodes[i]->public_key.bytes, pubnodes[j]->public_key.bytes, - 33) > 0) { - swap = pubnodes[i]; - pubnodes[i] = pubnodes[j]; - pubnodes[j] = swap; - } - } + + uint32_t pubkeys_order = multisig->has_pubkeys_order + ? multisig->pubkeys_order + : MultisigPubkeysOrder_PRESERVED; + + if (pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) { + // If the order of pubkeys is lexicographic, we don't want the fingerprint + // to depend on the order of the pubnodes, so we sort the pubnodes before + // hashing. + qsort(pubnodes, n, sizeof(HDNodeType *), comparePubnodesLexicographically); } - // hash sorted nodes + SHA256_CTX ctx = {0}; sha256_Init(&ctx); sha256_Update(&ctx, (const uint8_t *)&(multisig->m), sizeof(uint32_t)); + sha256_Update(&ctx, (const uint8_t *)&(pubkeys_order), sizeof(uint32_t)); for (uint32_t i = 0; i < n; i++) { sha256_Update(&ctx, (const uint8_t *)&(pubnodes[i]->depth), sizeof(uint32_t)); @@ -527,17 +582,7 @@ bool coin_path_check(const CoinInfo *coin, InputScriptType script_type, valid = valid && (address_n[2] <= PATH_MAX_CHANGE); valid = valid && (address_n[3] <= PATH_MAX_ADDRESS_INDEX); } else if (address_n_count == 5) { - if (address_n[1] & PATH_HARDENED) { - // 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_check); - valid = valid && (address_n[2] & PATH_HARDENED); - valid = - valid && ((address_n[2] & PATH_UNHARDEN_MASK) <= PATH_MAX_ACCOUNT); - valid = valid && (address_n[3] <= 1000000); - valid = valid && (address_n[4] <= PATH_MAX_ADDRESS_INDEX); - } else { + if ((address_n[1] & PATH_HARDENED) == 0) { // Casa proposed "universal multisig" pattern with unhardened parts. // m/45'/coin_type/account/change/address_index valid = valid && @@ -909,3 +954,27 @@ bool cryptoCosiVerify(const ed25519_signature signature, const uint8_t *message, res = ed25519_sign_open(message, message_len, pk_combined, signature); return res == 0; } + +bool multisig_uses_single_path(const MultisigRedeemScriptType *multisig) { + if (multisig->pubkeys_count == 0) { + // Pubkeys are specified by multisig.nodes and multisig.address_n, in this + // case all the pubkeys use the same path + return true; + } else { + // Pubkeys are specified by multisig.pubkeys, in this case we check that all + // the pubkeys use the same path + for (int i = 0; i < multisig->pubkeys_count; i++) { + if (multisig->pubkeys[i].address_n_count != + multisig->pubkeys[0].address_n_count) { + return false; + } + for (int j = 0; j < multisig->pubkeys[i].address_n_count; j++) { + if (multisig->pubkeys[i].address_n[j] != + multisig->pubkeys[0].address_n[j]) { + return false; + } + } + } + return true; + } +} diff --git a/legacy/firmware/crypto.h b/legacy/firmware/crypto.h index 8df938fc268..02c8566eaae 100644 --- a/legacy/firmware/crypto.h +++ b/legacy/firmware/crypto.h @@ -88,6 +88,14 @@ int cryptoMultisigPubkeyIndex(const CoinInfo *coin, const MultisigRedeemScriptType *multisig, const uint8_t *pubkey); +int cryptoMultisigXpubIndex(const CoinInfo *coin, + const MultisigRedeemScriptType *multisig, + const uint8_t *pubkey); + +uint32_t cryptoMultisigPubkeys(const CoinInfo *coin, + const MultisigRedeemScriptType *multisig, + uint8_t *pubkeys); + int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig, uint8_t *hash); @@ -115,5 +123,6 @@ void slip21_from_seed(const uint8_t *seed, int seed_len, Slip21Node *out); void slip21_derive_path(Slip21Node *inout, const uint8_t *label, size_t label_len); const uint8_t *slip21_key(const Slip21Node *node); +bool multisig_uses_single_path(const MultisigRedeemScriptType *multisig); #endif diff --git a/legacy/firmware/fsm.c b/legacy/firmware/fsm.c index 07c4c24b1cd..6bf1d0cb85a 100644 --- a/legacy/firmware/fsm.c +++ b/legacy/firmware/fsm.c @@ -442,6 +442,17 @@ bool fsm_layoutPathWarning(void) { return true; } +bool fsm_layoutDifferentPathsWarning(void) { + layoutDialogSwipe(&bmp_icon_warning, _("Abort"), _("Continue"), NULL, + _("Ussing different paths"), _("for different XPUBs."), + NULL, _("Continue at your"), _("own risk!"), NULL); + if (!protectButton(ButtonRequestType_ButtonRequest_Warning, false)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + return false; + } + return true; +} + #include "fsm_msg_coin.h" #include "fsm_msg_common.h" #include "fsm_msg_crypto.h" diff --git a/legacy/firmware/fsm.h b/legacy/firmware/fsm.h index 8b31c999148..37d2183c020 100644 --- a/legacy/firmware/fsm.h +++ b/legacy/firmware/fsm.h @@ -152,6 +152,7 @@ bool fsm_layoutSignMessage(const uint8_t *msg, uint32_t len); bool fsm_layoutVerifyMessage(const uint8_t *msg, uint32_t len); bool fsm_layoutPathWarning(void); +bool fsm_layoutDifferentPathsWarning(void); bool fsm_checkCoinPath(const CoinInfo *coin, InputScriptType script_type, uint32_t address_n_count, const uint32_t *address_n, bool has_multisig, MessageType message_type, diff --git a/legacy/firmware/fsm_msg_coin.h b/legacy/firmware/fsm_msg_coin.h index aae443a4233..66ae313f481 100644 --- a/legacy/firmware/fsm_msg_coin.h +++ b/legacy/firmware/fsm_msg_coin.h @@ -299,10 +299,37 @@ void fsm_msgGetAddress(const GetAddress *msg) { } if (msg->has_show_display && msg->show_display) { - char desc[20] = {0}; + char desc[29] = {0}; int multisig_index = 0; if (msg->has_multisig) { - strlcpy(desc, "Multisig __ of __:", sizeof(desc)); + if (!multisig_uses_single_path(&(msg->multisig))) { + // An address that uses different derivation paths for different xpubs + // could be difficult to discover if the user did not note all the + // paths. The reason is that each path ends with an address index, which + // can have 1,000,000 possible values. If the address is a t-out-of-n + // multisig, the total number of possible paths is 1,000,000^n. This can + // be exploited by an attacker who has compromised the user's computer. + // The attacker could randomize the address indices and then demand a + // ransom from the user to reveal the paths. To prevent this, we require + // that all xpubs use the same derivation path. + if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict) { + fsm_sendFailure( + FailureType_Failure_DataError, + _("Using different paths for different xpubs is not allowed")); + layoutHome(); + return; + } + if (!fsm_layoutDifferentPathsWarning()) { + layoutHome(); + return; + } + } + if (msg->multisig.has_pubkeys_order && + msg->multisig.pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) { + strlcpy(desc, "Multisig __ of __ (sorted):", sizeof(desc)); + } else { + strlcpy(desc, "Multisig __ of __:", sizeof(desc)); + } const uint32_t m = msg->multisig.m; const uint32_t n = cryptoMultisigPubkeyCount(&(msg->multisig)); desc[9] = (m < 10) ? ' ' : ('0' + (m / 10)); @@ -310,7 +337,7 @@ void fsm_msgGetAddress(const GetAddress *msg) { desc[15] = (n < 10) ? ' ' : ('0' + (n / 10)); desc[16] = '0' + (n % 10); multisig_index = - cryptoMultisigPubkeyIndex(coin, &(msg->multisig), node->public_key); + cryptoMultisigXpubIndex(coin, &(msg->multisig), node->public_key); } else { strlcpy(desc, _("Address:"), sizeof(desc)); } diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index 4d7d700e155..917c1abfd60 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -124,6 +124,8 @@ typedef struct { bool multisig_fp_set; bool multisig_fp_mismatch; uint8_t multisig_fp[32]; + bool is_multisig; + MatchState is_multisig_state; uint32_t in_address_n[8]; size_t in_address_n_count; InputScriptType in_script_type; @@ -978,6 +980,22 @@ static bool extract_input_multisig_fp(TxInfo *tx_info, return true; } +static void extract_is_multisig(TxInfo *tx_info, const TxInputType *txinput) { + switch (tx_info->is_multisig_state) { + case MatchState_MISMATCH: + return; + case MatchState_UNDEFINED: + tx_info->is_multisig_state = MatchState_MATCH; + tx_info->is_multisig = txinput->has_multisig; + return; + case MatchState_MATCH: + if (txinput->has_multisig != tx_info->is_multisig) { + tx_info->is_multisig_state = MatchState_MISMATCH; + } + return; + } +} + bool check_change_multisig_fp(const TxInfo *tx_info, const TxOutputType *txoutput) { uint8_t h[32] = {0}; @@ -986,6 +1004,12 @@ bool check_change_multisig_fp(const TxInfo *tx_info, memcmp(tx_info->multisig_fp, h, 32) == 0; } +bool check_change_is_multisig(const TxInfo *tx_info, + const TxOutputType *txoutput) { + return tx_info->is_multisig_state == MatchState_MATCH && + tx_info->is_multisig == txoutput->has_multisig; +} + static InputScriptType simple_script_type(InputScriptType script_type) { // SPENDMULTISIG is used only for non-SegWit and is effectively the same as // SPENDADDRESS. For SegWit inputs and outputs multisig is indicated by the @@ -1245,6 +1269,7 @@ static bool tx_info_init(TxInfo *tx_info, uint32_t inputs_count, tx_info->min_sequence = SEQUENCE_FINAL; tx_info->multisig_fp_set = false; tx_info->multisig_fp_mismatch = false; + tx_info->is_multisig_state = MatchState_UNDEFINED; tx_info->in_address_n_count = 0; tx_info->in_script_type = 0; tx_info->in_script_type_state = MatchState_UNDEFINED; @@ -1711,6 +1736,10 @@ static bool tx_info_add_input(TxInfo *tx_info, const TxInputType *txinput) { return false; } + // Remember whether all inputs are singlesig or all inputs are multisig. + // Change-outputs must be of the same type. + extract_is_multisig(tx_info, txinput); + // Remember the input's script type. Change-outputs must use the same script // type as all inputs. extract_input_script_type(tx_info, txinput); @@ -2069,15 +2098,25 @@ static bool is_change_output(const TxInfo *tx_info, return false; } - /* - * Check the multisig fingerprint only for multisig outputs. This means that - * a transfer from a multisig account to a singlesig account is treated as a - * change-output as long as all other change-output conditions are satisfied. - * This goes a bit against the concept of a multisig account, but the other - * cosigners will notice that they are relinquishing control of the funds, so - * there is no security risk. - */ - if (txoutput->has_multisig && !check_change_multisig_fp(tx_info, txoutput)) { + if (txoutput->has_multisig) { + if (!check_change_multisig_fp(tx_info, txoutput)) { + return false; + } + if (!multisig_uses_single_path(&(txoutput->multisig))) { + // An address that uses different derivation paths for different xpubs + // could be difficult to discover if the user did not note all the paths. + // The reason is that each path ends with an address index, which can have + // 1,000,000 possible values. If the address is a t-out-of-n multisig, the + // total number of possible paths is 1,000,000^n. This can be exploited by + // an attacker who has compromised the user's computer. The attacker could + // randomize the address indices and then demand a ransom from the user to + // reveal the paths. To prevent this, we require that all xpubs use the + // same derivation path. + return false; + } + } + + if (!check_change_is_multisig(tx_info, txoutput)) { return false; } diff --git a/legacy/firmware/transaction.c b/legacy/firmware/transaction.c index 8659dbf4b93..19dc54cf052 100644 --- a/legacy/firmware/transaction.c +++ b/legacy/firmware/transaction.c @@ -364,15 +364,16 @@ uint32_t compile_script_sig(uint32_t address_type, const uint8_t *pubkeyhash, uint32_t compile_script_multisig(const CoinInfo *coin, const MultisigRedeemScriptType *multisig, uint8_t *out) { - if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) { - fsm_sendFailure(FailureType_Failure_DataError, - _("Sortedmulti is not supported")); - return 0; - } const uint32_t m = multisig->m; const uint32_t n = cryptoMultisigPubkeyCount(multisig); if (m < 1 || m > 15) return 0; if (n < 1 || n > 15) return 0; + + uint8_t pubkeys[33 * n]; + if (!cryptoMultisigPubkeys(coin, multisig, pubkeys)) { + return 0; + } + uint32_t r = 0; if (out) { out[r] = 0x50 + m; @@ -380,9 +381,7 @@ uint32_t compile_script_multisig(const CoinInfo *coin, for (uint32_t i = 0; i < n; i++) { out[r] = 33; r++; // OP_PUSH 33 - const HDNode *pubnode = cryptoMultisigPubkey(coin, multisig, i); - if (!pubnode) return 0; - memcpy(out + r, pubnode->public_key, 33); + memcpy(out + r, pubkeys + 33 * i, 33); r += 33; } out[r] = 0x50 + n; @@ -398,17 +397,17 @@ uint32_t compile_script_multisig(const CoinInfo *coin, uint32_t compile_script_multisig_hash(const CoinInfo *coin, const MultisigRedeemScriptType *multisig, uint8_t *hash) { - if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) { - fsm_sendFailure(FailureType_Failure_DataError, - _("Sortedmulti is not supported")); - return 0; - } - const uint32_t m = multisig->m; const uint32_t n = cryptoMultisigPubkeyCount(multisig); if (m < 1 || m > 15) return 0; if (n < 1 || n > 15) return 0; + // allocate on stack instead of heap + uint8_t pubkeys[33 * n]; + if (!cryptoMultisigPubkeys(coin, multisig, pubkeys)) { + return 0; + } + Hasher hasher = {0}; hasher_Init(&hasher, coin->curve->hasher_script); @@ -418,9 +417,7 @@ uint32_t compile_script_multisig_hash(const CoinInfo *coin, for (uint32_t i = 0; i < n; i++) { d[0] = 33; hasher_Update(&hasher, d, 1); // OP_PUSH 33 - const HDNode *pubnode = cryptoMultisigPubkey(coin, multisig, i); - if (!pubnode) return 0; - hasher_Update(&hasher, pubnode->public_key, 33); + hasher_Update(&hasher, pubkeys + 33 * i, 33); } d[0] = 0x50 + n; d[1] = 0xAE; @@ -449,11 +446,6 @@ uint32_t serialize_script_sig(const uint8_t *signature, uint32_t signature_len, uint32_t serialize_script_multisig(const CoinInfo *coin, const MultisigRedeemScriptType *multisig, uint8_t sighash, uint8_t *out) { - if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) { - fsm_sendFailure(FailureType_Failure_DataError, - _("Sortedmulti is not supported")); - return 0; - } uint32_t r = 0; #if !BITCOIN_ONLY if (!coin->decred) { diff --git a/tests/device_tests/bitcoin/test_getaddress.py b/tests/device_tests/bitcoin/test_getaddress.py index 595735002fc..5367bcbb3e6 100644 --- a/tests/device_tests/bitcoin/test_getaddress.py +++ b/tests/device_tests/bitcoin/test_getaddress.py @@ -197,7 +197,6 @@ def test_altcoin_address_mac(client: Client): @pytest.mark.multisig -@pytest.mark.models(skip="legacy", reason="Sortedmulti is not supported") def test_multisig_pubkeys_order(client: Client): xpub_internal = btc.get_public_node(client, parse_path("m/45h/0")).xpub xpub_external = btc.get_public_node(client, parse_path("m/45h/1")).xpub @@ -438,7 +437,6 @@ def test_crw(client: Client): @pytest.mark.multisig -@pytest.mark.models(skip="legacy", reason="Not fixed") def test_multisig_different_paths(client: Client): nodes = [ btc.get_public_node(client, parse_path(f"m/45h/{i}"), coin_name="Bitcoin").node diff --git a/tests/device_tests/bitcoin/test_multisig.py b/tests/device_tests/bitcoin/test_multisig.py index 223afb07665..2a01db8108f 100644 --- a/tests/device_tests/bitcoin/test_multisig.py +++ b/tests/device_tests/bitcoin/test_multisig.py @@ -162,7 +162,6 @@ def test_2_of_3(client: Client, chunkify: bool): @pytest.mark.multisig -@pytest.mark.models(skip="legacy", reason="Sortedmulti is not supported") def test_pubkeys_order(client: Client): node_internal = btc.get_public_node( client, parse_path("m/45h/0"), coin_name="Bitcoin" diff --git a/tests/device_tests/bitcoin/test_multisig_change.py b/tests/device_tests/bitcoin/test_multisig_change.py index 9703a9b6727..7beaa31badc 100644 --- a/tests/device_tests/bitcoin/test_multisig_change.py +++ b/tests/device_tests/bitcoin/test_multisig_change.py @@ -83,6 +83,30 @@ m=2, ) +multisig_in4 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT1, NODE_EXT2, NODE_INT], + address_n=[0, 0], + signatures=[b"", b"", b""], + m=2, + pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC, +) + +multisig_in5 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT2, NODE_EXT1, NODE_INT], + address_n=[0, 1], + signatures=[b"", b"", b""], + m=2, + pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC, +) + +multisig_in6 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT1, NODE_EXT3, NODE_INT], + address_n=[0, 1], + signatures=[b"", b"", b""], + m=2, + pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC, +) + prev_hash_1, prev_tx_1 = forge_prevtx( [("3Ltgk5WPUMLcT2QvwRXKj9CWsYuAKqeHJ8", 50_000_000)] ) @@ -119,7 +143,51 @@ multisig=multisig_in3, ) -TX_API = {prev_hash_1: prev_tx_1, prev_hash_2: prev_tx_2, prev_hash_3: prev_tx_3} +prev_hash_4, prev_tx_4 = forge_prevtx( + [("3HwrvQEfYw4wUvGHpGmixWB15HPgqrvTh1", 50_000_000)] +) +INP4 = messages.TxInputType( + address_n=[H_(45), 0, 0, 0], + amount=50_000_000, + prev_hash=prev_hash_4, + prev_index=0, + script_type=messages.InputScriptType.SPENDMULTISIG, + multisig=multisig_in4, +) + +prev_hash_5, prev_tx_5 = forge_prevtx( + [("3Md42fbNjSH3qwnj5jDvT6CSzJKVXHiXSc", 34_500_000)] +) +INP5 = messages.TxInputType( + address_n=[H_(45), 0, 0, 1], + amount=34_500_000, + prev_hash=prev_hash_5, + prev_index=0, + script_type=messages.InputScriptType.SPENDMULTISIG, + multisig=multisig_in5, +) + +prev_hash_6, prev_tx_6 = forge_prevtx( + [("35PBSvszuvxhEDypGYcUhEQDigvKY8C5Rc", 55_500_000)] +) +INP6 = messages.TxInputType( + address_n=[H_(45), 0, 0, 1], + amount=55_500_000, + prev_hash=prev_hash_6, + prev_index=0, + script_type=messages.InputScriptType.SPENDMULTISIG, + multisig=multisig_in6, +) + + +TX_API = { + prev_hash_1: prev_tx_1, + prev_hash_2: prev_tx_2, + prev_hash_3: prev_tx_3, + prev_hash_4: prev_tx_4, + prev_hash_5: prev_tx_5, + prev_hash_6: prev_tx_6, +} def _responses( @@ -226,7 +294,7 @@ def test_external_internal(client: Client): client, INP1, INP2, - change_indices=[] if is_core(client) else [2], + change_indices=[], foreign_indices=[2], ) ) @@ -262,7 +330,7 @@ def test_internal_external(client: Client): client, INP1, INP2, - change_indices=[] if is_core(client) else [1], + change_indices=[], foreign_indices=[1], ) ) @@ -373,10 +441,46 @@ def test_multisig_change_match_second(client: Client): ) -# inputs match, change mismatches (second tries to be change but isn't) +# inputs match, change matches (first is change) +def test_sorted_multisig_change_match_first(client: Client): + multisig_out1 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT1, NODE_INT, NODE_EXT2], + address_n=[1, 0], + signatures=[b"", b"", b""], + m=2, + pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC, + ) + + out1 = messages.TxOutputType( + address_n=[H_(45), 0, 1, 0], + multisig=multisig_out1, + amount=40_000_000, + script_type=messages.OutputScriptType.PAYTOMULTISIG, + ) + + out2 = messages.TxOutputType( + address="3PkXLsY7AUZCrCKGvX8FfP2EawowUBMbcg", + amount=44_000_000, + script_type=messages.OutputScriptType.PAYTOADDRESS, + ) + + with client: + client.set_expected_responses( + _responses(client, INP4, INP5, change_indices=[1]) + ) + btc.sign_tx( + client, + "Bitcoin", + [INP4, INP5], + [out1, out2], + prev_txes=TX_API, + ) + + +# inputs match, change mismatches (second tries to be change but isn't because the pubkeys are in different order) def test_multisig_mismatch_multisig_change(client: Client): multisig_out2 = messages.MultisigRedeemScriptType( - nodes=[NODE_EXT1, NODE_INT, NODE_EXT3], + nodes=[NODE_EXT1, NODE_INT, NODE_EXT2], address_n=[1, 0], signatures=[b"", b"", b""], m=2, @@ -406,8 +510,40 @@ def test_multisig_mismatch_multisig_change(client: Client): ) -# inputs match, change mismatches (second tries to be change but isn't) -@pytest.mark.models(skip="legacy", reason="Not fixed") +# inputs match, change mismatches (second tries to be change but isn't because the pubkeys are different) +def test_sorted_multisig_mismatch_multisig_change(client: Client): + multisig_out2 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT1, NODE_INT, NODE_EXT3], + address_n=[1, 0], + signatures=[b"", b"", b""], + m=2, + ) + + out1 = messages.TxOutputType( + address="3B23k4kFBRtu49zvpG3Z9xuFzfpHvxBcwt", + amount=40_000_000, + script_type=messages.OutputScriptType.PAYTOADDRESS, + ) + + out2 = messages.TxOutputType( + address_n=[H_(45), 0, 1, 0], + multisig=multisig_out2, + amount=44_000_000, + script_type=messages.OutputScriptType.PAYTOMULTISIG, + ) + + with client: + client.set_expected_responses(_responses(client, INP4, INP5)) + btc.sign_tx( + client, + "Bitcoin", + [INP4, INP5], + [out1, out2], + prev_txes=TX_API, + ) + + +# inputs match, change mismatches (second tries to be change but isn't because is uses per-node paths) def test_multisig_mismatch_multisig_change_different_paths(client: Client): multisig_out2 = messages.MultisigRedeemScriptType( pubkeys=[ @@ -443,10 +579,10 @@ def test_multisig_mismatch_multisig_change_different_paths(client: Client): ) -# inputs mismatch, change matches with first input +# inputs mismatch because the pubkeys are different, change matches with first input def test_multisig_mismatch_inputs(client: Client): multisig_out1 = messages.MultisigRedeemScriptType( - nodes=[NODE_EXT2, NODE_EXT1, NODE_INT], + nodes=[NODE_EXT1, NODE_EXT2, NODE_INT], address_n=[1, 0], signatures=[b"", b"", b""], m=2, @@ -474,3 +610,37 @@ def test_multisig_mismatch_inputs(client: Client): [out1, out2], prev_txes=TX_API, ) + + +# inputs mismatch because the pubkeys are different, change matches with first input +def test_sorted_multisig_mismatch_inputs(client: Client): + multisig_out1 = messages.MultisigRedeemScriptType( + nodes=[NODE_EXT1, NODE_EXT2, NODE_INT], + address_n=[1, 0], + signatures=[b"", b"", b""], + m=2, + pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC, + ) + + out1 = messages.TxOutputType( + address_n=[H_(45), 0, 1, 0], + multisig=multisig_out1, + amount=40_000_000, + script_type=messages.OutputScriptType.PAYTOMULTISIG, + ) + + out2 = messages.TxOutputType( + address="3PkXLsY7AUZCrCKGvX8FfP2EawowUBMbcg", + amount=65_000_000, + script_type=messages.OutputScriptType.PAYTOADDRESS, + ) + + with client: + client.set_expected_responses(_responses(client, INP4, INP6)) + btc.sign_tx( + client, + "Bitcoin", + [INP4, INP6], + [out1, out2], + prev_txes=TX_API, + ) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index a47cad6abe9..a87082ccc99 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -60,8 +60,10 @@ "T1B1_en_bitcoin-test_getaddress.py::test_invalid_path": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_ltc": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_multisig": "09d812e81c04cef37608f181dcdefb6cd186eb5bb111d47ae0e62d1b3fdd64cf", +"T1B1_en_bitcoin-test_getaddress.py::test_multisig_different_paths": "bcecf88ef3112f415545d37a036627823c426f774c58cca1c91f11c5ad953a5d", "T1B1_en_bitcoin-test_getaddress.py::test_multisig_missing[False]": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_multisig_missing[True]": "ef912e3aed3113ed37f982568c0eca17c27feeb67e87e0f7e43cf8b3b3e8d199", +"T1B1_en_bitcoin-test_getaddress.py::test_multisig_pubkeys_order": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_public_ckd": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_tbtc": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "T1B1_en_bitcoin-test_getaddress.py::test_tgrs": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", @@ -176,14 +178,19 @@ "T1B1_en_bitcoin-test_multisig.py::test_2_of_3[True]": "ce96277e0a7b026144ee6cdd501c160c27a6d4a2ad8451d9f4de8114eebe0cde", "T1B1_en_bitcoin-test_multisig.py::test_attack_change_input": "d8731108a403d5853de526b27e506d00909603ec0c89d1c20f917ca2ef012ab3", "T1B1_en_bitcoin-test_multisig.py::test_missing_pubkey": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", +"T1B1_en_bitcoin-test_multisig.py::test_pubkeys_order": "5a84d659899e67ff72b206392d53f1418aec1fa9176e251c0398cf7347634baf", "T1B1_en_bitcoin-test_multisig_change.py::test_external_external": "e4a9c75e35046d5605d3663717ec025e7c7919ce757cdd9df66eba3bd7790d1d", -"T1B1_en_bitcoin-test_multisig_change.py::test_external_internal": "c3cc02dc9d002d8f392887248348fce3322f0fea2860d755a08fe5d9f7b21ffc", -"T1B1_en_bitcoin-test_multisig_change.py::test_internal_external": "7a3a1e10b1b14561e2afb63650c1a6273e05232e90bfba15fd0f1bdb131c7d51", +"T1B1_en_bitcoin-test_multisig_change.py::test_external_internal": "4a3581a38cb3c5afafd92deb8d0b4b567191541816ca19c609eafd0820b100cc", +"T1B1_en_bitcoin-test_multisig_change.py::test_internal_external": "41971f517347edc9eb36c76b0986e78620ba4bc7fbb9d7bbb3081b95a788b4d7", "T1B1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_first": "602f0669af9084a07900170c00699512d1cd4646aba7ddcebf6af05e36eb224b", "T1B1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_second": "ee110f116e966f842ffc174e3ab55e951aec31b1f0433fc697c9d7aaed3d7060", "T1B1_en_bitcoin-test_multisig_change.py::test_multisig_external_external": "0c398595da3162afee0f4734a155c0cf8d96b830350b58670b47aba40659e3cc", -"T1B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "601dbfefbebf6dcf988a3c1c5508dc75c59df753b0788a2f017eb4346f3ab55c", -"T1B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "25d6670f1a30d56fccf78d3119aaa876b5f31655b6901a3a4ef1838748045cae", +"T1B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "8143cb729d0c000809a537274cdb461840339e6aa64dd6adcbb7cf17fbd698eb", +"T1B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "054a78485f6241e44946c7e340602893bd2045b812d8cd72ae2fc573f017c649", +"T1B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change_different_paths": "bb85ac385e45da370102a95f116c304d88ec0cfbf9bacf3014a83f3bee9b6a6d", +"T1B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_change_match_first": "602f0669af9084a07900170c00699512d1cd4646aba7ddcebf6af05e36eb224b", +"T1B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_inputs": "601dbfefbebf6dcf988a3c1c5508dc75c59df753b0788a2f017eb4346f3ab55c", +"T1B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_multisig_change": "25d6670f1a30d56fccf78d3119aaa876b5f31655b6901a3a4ef1838748045cae", "T1B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-False]": "c968d01488a358a7287c4e631996081bb490ff6e702b5ff84c3addba1c7b0974", "T1B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-True]": "c968d01488a358a7287c4e631996081bb490ff6e702b5ff84c3addba1c7b0974", "T1B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-script_types2-False]": "814c415655b03a6757056456544852a7ec88f8e96fbc97728eba7874d03782f8", @@ -4170,9 +4177,12 @@ "T2T1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_first": "abac1bac2d30a9b340608f6b237db811204d77c4603b12190a8818030d4537d2", "T2T1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_second": "2940a8795f35c1b0b0766746f1751861cc7830c7ed4d7a3a069ab429e61e7e65", "T2T1_en_bitcoin-test_multisig_change.py::test_multisig_external_external": "9ac2072a8569d3cffce8934241cfed507a5499ea9f6a5168712c87e2c1f45475", -"T2T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "ac263c92006cfc66dec85f1b4e476776bbc350a683bee83817262702d467d247", -"T2T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "6259078a9471958e79ddf36ab36439eb779f5a773bdbff9edabf2a20e0b5e765", +"T2T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "209f87af11dbf117f92540a63df1e81e8e2b5c7016d6433b36b0bc873e3ad0a4", +"T2T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "cc943279896812ad7e33255cc518c328903a3395560f6a3daf277ba56c76916f", "T2T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change_different_paths": "6ecf3440429cd290284861891a1cbacb916b9a2fe92116f573bdc55746975f06", +"T2T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_change_match_first": "abac1bac2d30a9b340608f6b237db811204d77c4603b12190a8818030d4537d2", +"T2T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_inputs": "ac263c92006cfc66dec85f1b4e476776bbc350a683bee83817262702d467d247", +"T2T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_multisig_change": "6259078a9471958e79ddf36ab36439eb779f5a773bdbff9edabf2a20e0b5e765", "T2T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-False]": "aa185dc27f19708ea687c235b1d1a2d3d7ff109438c23dd69840e53b47fc62c8", "T2T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-True]": "ef46aae67230f4ff7436241f25ec676ba9edec5dcca4a2a273609ea65d95c902", "T2T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-script_types2-False]": "de5af7f97f09a852c87c1850675596395b28188c11933310bf0c175b44be5d02", @@ -13024,9 +13034,12 @@ "T3B1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_first": "6f71500921c5c447e856621718773b185bde610eed3493ad9b6e0ebe1b525702", "T3B1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_second": "42dc29edd022916630bc060a1e4cddf4bedac0919bcea75506e2b7dfdabdefc0", "T3B1_en_bitcoin-test_multisig_change.py::test_multisig_external_external": "fc8d123151f10d1f2078d57051bf1521ba47b50e8bc333627979d5f7225e15ce", -"T3B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "2f889a87a24b2a287a18c1530f24185b3b5dc731715ed2f0d2efd5b2c72a74a8", -"T3B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "4823625210a536a6397621e9a3e1a4de88b296a6fb088f4dd165bc9b5660fdef", +"T3B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "bf3f0f0697a93bbd8c73f1ecd3f6bad24adcf24e0e7d9eb992232f530cc99f9e", +"T3B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "bfabfe62a724ca2a7748c4561832d8dc9fdcaadbce1e07ebe6fd4ead0f73d95c", "T3B1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change_different_paths": "2d590435b83ed8606d53d936ce6bad790c5c8807be953e5f57447c3fdc30644f", +"T3B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_change_match_first": "6f71500921c5c447e856621718773b185bde610eed3493ad9b6e0ebe1b525702", +"T3B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_inputs": "2f889a87a24b2a287a18c1530f24185b3b5dc731715ed2f0d2efd5b2c72a74a8", +"T3B1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_multisig_change": "4823625210a536a6397621e9a3e1a4de88b296a6fb088f4dd165bc9b5660fdef", "T3B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-False]": "b943138aee326039c2265bf41bbfdf769929db659339fbb0789079083a59ca38", "T3B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-True]": "551bbe9d3ef36de8bde8359f5e00f07487a7a6a38960cef1a798f5c636d8dd54", "T3B1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-script_types2-False]": "c84f6746a9ff93bc9d0ba310a957e698834a176b329f8c1b05d85c319560a490", @@ -21634,9 +21647,12 @@ "T3T1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_first": "76118c3ff22ad95b1a6078a28be695f5c3687ef62cc3a5886c71fa08e3c11c12", "T3T1_en_bitcoin-test_multisig_change.py::test_multisig_change_match_second": "8e5a353a7fac88e64e221b6f3707a252b9291253149c30b44c6ef8d726e1ef1b", "T3T1_en_bitcoin-test_multisig_change.py::test_multisig_external_external": "2fb40dffc372a78cc536f9dcf7465acda8fc6495ddc3727c55114ea94425ced3", -"T3T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "12b162a58c7953259a976a12d02c0dc6379575f67d97724632ccff4fc32e0f2e", -"T3T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "8124d8dc6f2c1ac4a76b0c651a7f4a053bc725aa33d4462b883b1a0bf9cd16c2", +"T3T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_inputs": "0aede16b87bd2c06ab07a7e02a615f0187c4f4c0429083379894f2e00f3cf66a", +"T3T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change": "caaebceba418a3570d8e2e246f01119ae7f9f00bc10510fe43e01ec34787c00a", "T3T1_en_bitcoin-test_multisig_change.py::test_multisig_mismatch_multisig_change_different_paths": "f9064a9529f63e61f966e68e15f6531cc5cef3a0310ceb57c9a0ec0b72b6549d", +"T3T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_change_match_first": "76118c3ff22ad95b1a6078a28be695f5c3687ef62cc3a5886c71fa08e3c11c12", +"T3T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_inputs": "12b162a58c7953259a976a12d02c0dc6379575f67d97724632ccff4fc32e0f2e", +"T3T1_en_bitcoin-test_multisig_change.py::test_sorted_multisig_mismatch_multisig_change": "8124d8dc6f2c1ac4a76b0c651a7f4a053bc725aa33d4462b883b1a0bf9cd16c2", "T3T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-False]": "dc68c9455d872221e5d69c361ac9938444e1b1d9eada4f84508e18b3f8b2a74d", "T3T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-6-255-script_types3-True]": "7adbeb81cfa5e49877e240fdf003b6ee71fb50c9c2e018c4abd98b654804b5bc", "T3T1_en_bitcoin-test_nonstandard_paths.py::test_getaddress[m-1195487518-script_types2-False]": "696de2cb811b029f6e68276cfec69e5f13b8b149dce8fd1ad92dd5ba7a6a8368",