aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/consensus/tx_check.cpp19
-rw-r--r--src/consensus/tx_check.h2
-rw-r--r--src/test/fuzz/transaction.cpp7
-rw-r--r--src/validation.cpp3
4 files changed, 13 insertions, 18 deletions
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index 1206035839..6793f871cf 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -7,7 +7,7 @@
#include <primitives/transaction.h>
#include <consensus/validation.h>
-bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fCheckDuplicateInputs)
+bool CheckTransaction(const CTransaction& tx, CValidationState& state)
{
// Basic checks that don't depend on any context
if (tx.vin.empty())
@@ -31,14 +31,15 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-txouttotal-toolarge");
}
- // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
- if (fCheckDuplicateInputs) {
- std::set<COutPoint> vInOutPoints;
- for (const auto& txin : tx.vin)
- {
- if (!vInOutPoints.insert(txin.prevout).second)
- return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
- }
+ // Check for duplicate inputs (see CVE-2018-17144)
+ // While Consensus::CheckTxInputs does check if all inputs of a tx are available, and UpdateCoins marks all inputs
+ // of a tx as spent, it does not check if the tx has duplicate inputs.
+ // Failure to run this check will result in either a crash or an inflation bug, depending on the implementation of
+ // the underlying coins database.
+ std::set<COutPoint> vInOutPoints;
+ for (const auto& txin : tx.vin) {
+ if (!vInOutPoints.insert(txin.prevout).second)
+ return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-txns-inputs-duplicate");
}
if (tx.IsCoinBase())
diff --git a/src/consensus/tx_check.h b/src/consensus/tx_check.h
index bcfdf36bf9..6f3f8fe969 100644
--- a/src/consensus/tx_check.h
+++ b/src/consensus/tx_check.h
@@ -15,6 +15,6 @@
class CTransaction;
class CValidationState;
-bool CheckTransaction(const CTransaction& tx, CValidationState& state, bool fCheckDuplicateInputs=true);
+bool CheckTransaction(const CTransaction& tx, CValidationState& state);
#endif // BITCOIN_CONSENSUS_TX_CHECK_H
diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp
index 96d7947b07..383d879040 100644
--- a/src/test/fuzz/transaction.cpp
+++ b/src/test/fuzz/transaction.cpp
@@ -43,12 +43,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
}
CValidationState state_with_dupe_check;
- const bool valid_with_dupe_check = CheckTransaction(tx, state_with_dupe_check, /* fCheckDuplicateInputs= */ true);
- CValidationState state_without_dupe_check;
- const bool valid_without_dupe_check = CheckTransaction(tx, state_without_dupe_check, /* fCheckDuplicateInputs= */ false);
- if (valid_with_dupe_check) {
- assert(valid_without_dupe_check);
- }
+ (void)CheckTransaction(tx, state_with_dupe_check);
const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE};
std::string reason;
diff --git a/src/validation.cpp b/src/validation.cpp
index 70b847d3b0..f1abcadefc 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3301,9 +3301,8 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
return state.Invalid(ValidationInvalidReason::CONSENSUS, false, "bad-cb-multiple", "more than one coinbase");
// Check transactions
- // Must check for duplicate inputs (see CVE-2018-17144)
for (const auto& tx : block.vtx)
- if (!CheckTransaction(*tx, state, true))
+ if (!CheckTransaction(*tx, state))
return state.Invalid(state.GetReason(), false, state.GetRejectReason(),
strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage()));