phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-29 14:59:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#32122: fuzz: Fix off-by-one in package_rbf target
Some checks are pending
CI / test each commit (push) Waiting to run
Details
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
Details
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
Details
CI / Win64 native, VS 2022 (push) Waiting to run
Details
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
Details
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
Details

Browse source 
fa5674c264 fuzz: Fix off-by-one in package_rbf target (MarcoFalke)

Pull request description:

  Running the while loop up to `NUM_ITERS` times may set `iter` to `g_outpoints.size()`, which will then lead to an out-of-bounds read.

  There was an assert, which I guess tried to catch this, but the condition in the assert was wrong as well.

  Fix all issues by replacing the broken assert with the internal and correct check inside `std::vector::at` and by limiting `iter` to `NUM_ITERS` in the while loop.

  Fixes https://github.com/bitcoin/bitcoin/issues/32121

ACKs for top commit:
  glozow:
    ACK fa5674c264
  brunoerg:
    code review ACK fa5674c264

Tree-SHA512: 91b849ce969fd25c0ff8c90c2908d3096c77607d8e5fd81201ef33d88a57760199618174b8a6fd634cb5ef2a9068e94703b5c963ca473bd96a14d4bf9ec835cb
This commit is contained in:
merge-script 2025-03-25 16:57:38 -04:00
commit c0b7159de4
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4
1 changed files with 7 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/test/fuzz/rbf.cpp
View file

@ -1,4 +1,4 @@
// Copyright (c) 2020-2022 The Bitcoin Core developers // Copyright (c) 2020-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying // Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php. // file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -108,7 +108,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
// Add a bunch of parent-child pairs to the mempool, and remember them. // Add a bunch of parent-child pairs to the mempool, and remember them.
std::vector<CTransaction> mempool_txs; std::vector<CTransaction> mempool_txs;
size_t iter{0}; uint32_t iter{0};
// Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow
// Add replacement_vsize since this is added to new diagram during RBF check // Add replacement_vsize since this is added to new diagram during RBF check
@ -116,9 +116,8 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
if (!replacement_tx) { if (!replacement_tx) {
return; return;
} }
assert(iter <= g_outpoints.size());
replacement_tx->vin.resize(1); replacement_tx->vin.resize(1);
replacement_tx->vin[0].prevout = g_outpoints[iter++]; replacement_tx->vin[0].prevout = g_outpoints.at(iter++);
CTransaction replacement_tx_final{*replacement_tx}; CTransaction replacement_tx_final{*replacement_tx};
auto replacement_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, replacement_tx_final); auto replacement_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, replacement_tx_final);
int32_t replacement_vsize = replacement_entry.GetTxSize(); int32_t replacement_vsize = replacement_entry.GetTxSize();
@ -126,13 +125,13 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
LOCK2(cs_main, pool.cs); LOCK2(cs_main, pool.cs);
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) while (fuzzed_data_provider.ConsumeBool()) {
{ if (iter >= NUM_ITERS) break;
// Make sure txns only have one input, and that a unique input is given to avoid circular references // Make sure txns only have one input, and that a unique input is given to avoid circular references
CMutableTransaction parent; CMutableTransaction parent;
assert(iter <= g_outpoints.size());
parent.vin.resize(1); parent.vin.resize(1);
parent.vin[0].prevout = g_outpoints[iter++]; parent.vin[0].prevout = g_outpoints.at(iter++);
parent.vout.emplace_back(0, CScript()); parent.vout.emplace_back(0, CScript());
mempool_txs.emplace_back(parent); mempool_txs.emplace_back(parent);