Skip to content

Commit c0b7159

Browse files
committed
Merge bitcoin/bitcoin#32122: fuzz: Fix off-by-one in package_rbf target
fa5674c 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 bitcoin/bitcoin#32121 ACKs for top commit: glozow: ACK fa5674c brunoerg: code review ACK fa5674c Tree-SHA512: 91b849ce969fd25c0ff8c90c2908d3096c77607d8e5fd81201ef33d88a57760199618174b8a6fd634cb5ef2a9068e94703b5c963ca473bd96a14d4bf9ec835cb
2 parents dfb7d58 + fa5674c commit c0b7159

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

src/test/fuzz/rbf.cpp

+7-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) 2020-2022 The Bitcoin Core developers
1+
// Copyright (c) 2020-present The Bitcoin Core developers
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

@@ -108,31 +108,30 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
108108

109109
// Add a bunch of parent-child pairs to the mempool, and remember them.
110110
std::vector<CTransaction> mempool_txs;
111-
size_t iter{0};
111+
uint32_t iter{0};
112112

113113
// Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow
114114
// Add replacement_vsize since this is added to new diagram during RBF check
115115
std::optional<CMutableTransaction> replacement_tx = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
116116
if (!replacement_tx) {
117117
return;
118118
}
119-
assert(iter <= g_outpoints.size());
120119
replacement_tx->vin.resize(1);
121-
replacement_tx->vin[0].prevout = g_outpoints[iter++];
120+
replacement_tx->vin[0].prevout = g_outpoints.at(iter++);
122121
CTransaction replacement_tx_final{*replacement_tx};
123122
auto replacement_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, replacement_tx_final);
124123
int32_t replacement_vsize = replacement_entry.GetTxSize();
125124
int64_t running_vsize_total{replacement_vsize};
126125

127126
LOCK2(cs_main, pool.cs);
128127

129-
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS)
130-
{
128+
while (fuzzed_data_provider.ConsumeBool()) {
129+
if (iter >= NUM_ITERS) break;
130+
131131
// Make sure txns only have one input, and that a unique input is given to avoid circular references
132132
CMutableTransaction parent;
133-
assert(iter <= g_outpoints.size());
134133
parent.vin.resize(1);
135-
parent.vin[0].prevout = g_outpoints[iter++];
134+
parent.vin[0].prevout = g_outpoints.at(iter++);
136135
parent.vout.emplace_back(0, CScript());
137136

138137
mempool_txs.emplace_back(parent);

0 commit comments

Comments
 (0)