diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index 4d7d700e15..f4ac2d3c47 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -124,6 +124,9 @@ typedef struct { bool multisig_fp_set; bool multisig_fp_mismatch; uint8_t multisig_fp[32]; + bool multisig; + bool multisig_set; + bool multisig_mismatch; uint32_t in_address_n[8]; size_t in_address_n_count; InputScriptType in_script_type; @@ -978,6 +981,22 @@ static bool extract_input_multisig_fp(TxInfo *tx_info, return true; } +static void extract_multisig(TxInfo *tx_info, const TxInputType *txinput) { + if (tx_info->multisig_mismatch) { + return; + } + + if (!tx_info->multisig_set) { + tx_info->multisig = txinput->has_multisig; + tx_info->multisig_set = true; + return; + } + + if (txinput->has_multisig != tx_info->multisig) { + tx_info->multisig_mismatch = true; + } +} + bool check_change_multisig_fp(const TxInfo *tx_info, const TxOutputType *txoutput) { uint8_t h[32] = {0}; @@ -986,6 +1005,12 @@ bool check_change_multisig_fp(const TxInfo *tx_info, memcmp(tx_info->multisig_fp, h, 32) == 0; } +bool check_change_multisig(const TxInfo *tx_info, + const TxOutputType *txoutput) { + return tx_info->multisig_set && !tx_info->multisig_mismatch && + txoutput->has_multisig == tx_info->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 +1270,8 @@ 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->multisig_set = false; + tx_info->multisig_mismatch = false; tx_info->in_address_n_count = 0; tx_info->in_script_type = 0; tx_info->in_script_type_state = MatchState_UNDEFINED; @@ -1711,6 +1738,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_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,18 +2100,14 @@ 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)) { return false; } + if (!check_change_multisig(tx_info, txoutput)) { + return false; + } + if (!check_change_script_type(tx_info, txoutput)) { return false; } diff --git a/tests/device_tests/bitcoin/test_multisig_change.py b/tests/device_tests/bitcoin/test_multisig_change.py index 9703a9b672..2becd97f67 100644 --- a/tests/device_tests/bitcoin/test_multisig_change.py +++ b/tests/device_tests/bitcoin/test_multisig_change.py @@ -226,7 +226,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 +262,7 @@ def test_internal_external(client: Client): client, INP1, INP2, - change_indices=[] if is_core(client) else [1], + change_indices=[], foreign_indices=[1], ) )