From 181fd1c6013d9ffdb11f11cd1aabaaa897845148 Mon Sep 17 00:00:00 2001 From: Andrew Kozlik Date: Fri, 11 Mar 2022 12:25:53 +0100 Subject: [PATCH] fix(legacy): Stricter Bitcoin transaction checks. [no changelog] --- legacy/firmware/signing.c | 30 +++++++++++++++++++++++++----- legacy/firmware/transaction.c | 29 ++++++++++++++++++++++------- legacy/firmware/transaction.h | 2 +- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c index 1b00442b7..4db1918c7 100644 --- a/legacy/firmware/signing.c +++ b/legacy/firmware/signing.c @@ -1370,7 +1370,12 @@ static void tx_info_finish(TxInfo *tx_info) { static bool signing_add_input(TxInputType *txinput) { // hash all input data to check it later (relevant for fee computation) - tx_input_check_hash(&hasher_check, txinput); + if (!tx_input_check_hash(&hasher_check, txinput)) { + fsm_sendFailure(FailureType_Failure_ProcessError, + _("Failed to hash input")); + signing_abort(); + return false; + } if (!fsm_checkCoinPath(coin, txinput->script_type, txinput->address_n_count, txinput->address_n, txinput->has_multisig, true)) { @@ -1634,8 +1639,13 @@ static bool signing_add_orig_input(TxInputType *orig_input) { return false; } - // The first original input that has address_n set and a signature gets chosen - // as the verification input. Set script_sig for legacy digest computation. + // The first original non-multisig input that has address_n set and a + // signature gets chosen as the verification input. Set script_sig for legacy + // digest computation. + // NOTE: Supporting replacement transactions where all internal inputs are + // multisig would require checking the signatures of all of the original + // internal inputs or not allowing unverified external inputs in transactions + // where multisig inputs are present. if (!have_orig_verif_input && orig_input->address_n_count != 0 && !orig_input->has_multisig && ((orig_input->has_script_sig && orig_input->script_sig.size != 0) || @@ -2736,7 +2746,12 @@ void signing_txack(TransactionType *tx) { if (idx1 == 0) { hasher_Reset(&hasher_check); } - tx_input_check_hash(&hasher_check, tx->inputs); + if (!tx_input_check_hash(&hasher_check, tx->inputs)) { + fsm_sendFailure(FailureType_Failure_ProcessError, + _("Failed to hash input")); + signing_abort(); + return; + } memcpy(&input, tx->inputs, sizeof(TxInputType)); @@ -2943,7 +2958,12 @@ void signing_txack(TransactionType *tx) { hasher_Reset(&hasher_check); } // check inputs are the same as those in phase 1 - tx_input_check_hash(&hasher_check, tx->inputs); + if (!tx_input_check_hash(&hasher_check, tx->inputs)) { + fsm_sendFailure(FailureType_Failure_ProcessError, + _("Failed to hash input")); + signing_abort(); + return; + } if (idx2 == idx1) { if (!tx_info_check_input(&info, &tx->inputs[0]) || !derive_node(&tx->inputs[0]) || diff --git a/legacy/firmware/transaction.c b/legacy/firmware/transaction.c index 49a9a9aea..f60a43c60 100644 --- a/legacy/firmware/transaction.c +++ b/legacy/firmware/transaction.c @@ -556,22 +556,37 @@ uint32_t serialize_script_multisig(const CoinInfo *coin, } // tx methods -void tx_input_check_hash(Hasher *hasher, const TxInputType *input) { - hasher_Update(hasher, input->prev_hash.bytes, sizeof(input->prev_hash.bytes)); - hasher_Update(hasher, (const uint8_t *)&input->prev_index, - sizeof(input->prev_index)); - hasher_Update(hasher, (const uint8_t *)&input->script_type, - sizeof(input->script_type)); +bool tx_input_check_hash(Hasher *hasher, const TxInputType *input) { hasher_Update(hasher, (const uint8_t *)&input->address_n_count, sizeof(input->address_n_count)); for (int i = 0; i < input->address_n_count; ++i) hasher_Update(hasher, (const uint8_t *)&input->address_n[i], sizeof(input->address_n[0])); + hasher_Update(hasher, input->prev_hash.bytes, sizeof(input->prev_hash.bytes)); + hasher_Update(hasher, (const uint8_t *)&input->prev_index, + sizeof(input->prev_index)); + tx_script_hash(hasher, input->script_sig.size, input->script_sig.bytes); hasher_Update(hasher, (const uint8_t *)&input->sequence, sizeof(input->sequence)); + hasher_Update(hasher, (const uint8_t *)&input->script_type, + sizeof(input->script_type)); + uint8_t multisig_fp[32] = {0}; + if (input->has_multisig) { + if (cryptoMultisigFingerprint(&input->multisig, multisig_fp) == 0) { + // Invalid multisig parameters. + return false; + } + } + hasher_Update(hasher, multisig_fp, sizeof(multisig_fp)); hasher_Update(hasher, (const uint8_t *)&input->amount, sizeof(input->amount)); + tx_script_hash(hasher, input->witness.size, input->witness.bytes); + hasher_Update(hasher, (const uint8_t *)&input->has_orig_hash, + sizeof(input->has_orig_hash)); + hasher_Update(hasher, input->orig_hash.bytes, sizeof(input->orig_hash.bytes)); + hasher_Update(hasher, (const uint8_t *)&input->orig_index, + sizeof(input->orig_index)); tx_script_hash(hasher, input->script_pubkey.size, input->script_pubkey.bytes); - return; + return true; } uint32_t tx_prevout_hash(Hasher *hasher, const TxInputType *input) { diff --git a/legacy/firmware/transaction.h b/legacy/firmware/transaction.h index 5d6094e0b..3929a72b3 100644 --- a/legacy/firmware/transaction.h +++ b/legacy/firmware/transaction.h @@ -79,7 +79,7 @@ int compile_output(const CoinInfo *coin, AmountUnit amount_unit, int fill_input_script_pubkey(const CoinInfo *coin, const HDNode *root, TxInputType *in); -void tx_input_check_hash(Hasher *hasher, const TxInputType *input); +bool tx_input_check_hash(Hasher *hasher, const TxInputType *input); uint32_t tx_prevout_hash(Hasher *hasher, const TxInputType *input); uint32_t tx_amount_hash(Hasher *hasher, const TxInputType *input); uint32_t tx_script_hash(Hasher *hasher, uint32_t size, const uint8_t *data);