diff options
author | fanquake <fanquake@gmail.com> | 2023-05-31 11:12:06 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-05-31 11:24:57 +0100 |
commit | 2a786ea3493c9c2c628137806271f77e62fbee05 (patch) | |
tree | ebca5e8f5964e465b81a064e5b13a8d5b275d7fa | |
parent | 1b8b28d83b2b59356fbe718c552156f60a951cf2 (diff) | |
parent | fafb4da121b19ba1b7bd173e25651c64d1982fb4 (diff) | |
download | bitcoin-2a786ea3493c9c2c628137806271f77e62fbee05.tar.xz |
Merge bitcoin/bitcoin#27780: fuzz: Avoid timeout in utxo_total_supply
fafb4da121b19ba1b7bd173e25651c64d1982fb4 fuzz: Avoid timeout in utxo_total_supply (MarcoFalke)
Pull request description:
Looks like for high block counts it may be better to mock the chain, otherwise a high limit will lead to fuzz input bloat and timeouts, see https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1538252773.
It can be checked that the fuzz target can still find the CVE, see https://github.com/bitcoin/bitcoin/pull/17860#pullrequestreview-1410594057 with a diff of:
```diff
diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index f949655909..6f4cfb5f51 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -39,8 +39,6 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state)
// the underlying coins database.
std::set<COutPoint> vInOutPoints;
for (const auto& txin : tx.vin) {
- if (!vInOutPoints.insert(txin.prevout).second)
- return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate");
}
if (tx.IsCoinBase())
```
Also, fix a nit, see https://github.com/bitcoin/bitcoin/pull/17860#discussion_r1186451948
ACKs for top commit:
dergoegge:
ACK fafb4da121b19ba1b7bd173e25651c64d1982fb4
Tree-SHA512: a28fe9cd6ebb4c9bed5a5b35be76c1c436a87586c8fc3b3c4c8559a4a77ac08098324370da421d794c99579882c0872b6b29415de47ade6a05a08504a3d494c4
-rw-r--r-- | src/test/fuzz/utxo_total_supply.cpp | 14 |
1 files changed, 8 insertions, 6 deletions
diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index 19f41880f4..ea78edd05f 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -119,7 +119,9 @@ FUZZ_TARGET(utxo_total_supply) current_block = PrepareNextBlock(); StoreLastTxo(); - LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 100'000) + // Limit to avoid timeout, but enough to cover duplicate_coinbase_height + // and CVE-2018-17144. + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 2'000) { CallOneOf( fuzzed_data_provider, @@ -142,14 +144,14 @@ FUZZ_TARGET(utxo_total_supply) node::RegenerateCommitments(*current_block, chainman); const bool was_valid = !MineBlock(node, current_block).IsNull(); + if (duplicate_coinbase_height == ActiveHeight()) { + // we mined the duplicate coinbase + assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); + } + const auto prev_utxo_stats = utxo_stats; if (was_valid) { circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus()); - - if (duplicate_coinbase_height == ActiveHeight()) { - // we mined the duplicate coinbase - assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); - } } UpdateUtxoStats(); |