Skip to content

Commit 01960c5

Browse files
martinusmartinus
committed
fuzz: make FuzzedDataProvider usage deterministic
There exist many usages of `fuzzed_data_provider` where it is evaluated directly in the function call. Unfortunately, the order of evaluation of function arguments is unspecified. This means it can differ between compilers/version/optimization levels etc. But when the evaluation order changes, the same fuzzing input will produce different output, which is bad for coverage/reproducibility. This PR fixes all these cases where by moving multiple calls to `fuzzed_data_provider` out of the function arguments.
1 parent 3e69125 commit 01960c5

18 files changed

+129
-66
lines changed

‎src/test/fuzz/addrman.cpp

+20-10
Original file line numberDiff line numberDiff line change
@@ -263,31 +263,41 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
263263
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
264264
addresses.push_back(ConsumeAddress(fuzzed_data_provider));
265265
}
266-
addr_man.Add(addresses, ConsumeNetAddr(fuzzed_data_provider), std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)});
266+
auto net_addr = ConsumeNetAddr(fuzzed_data_provider);
267+
auto time_penalty = std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)};
268+
addr_man.Add(addresses, net_addr, time_penalty);
267269
},
268270
[&] {
269-
addr_man.Good(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
271+
auto addr = ConsumeService(fuzzed_data_provider);
272+
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
273+
addr_man.Good(addr, time);
270274
},
271275
[&] {
272-
addr_man.Attempt(ConsumeService(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
276+
auto addr = ConsumeService(fuzzed_data_provider);
277+
auto count_failure = fuzzed_data_provider.ConsumeBool();
278+
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
279+
addr_man.Attempt(addr, count_failure, time);
273280
},
274281
[&] {
275-
addr_man.Connected(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
282+
auto addr = ConsumeService(fuzzed_data_provider);
283+
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
284+
addr_man.Connected(addr, time);
276285
},
277286
[&] {
278-
addr_man.SetServices(ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
287+
auto addr = ConsumeService(fuzzed_data_provider);
288+
auto n_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
289+
addr_man.SetServices(addr, n_services);
279290
});
280291
}
281292
const AddrMan& const_addr_man{addr_man};
282293
std::optional<Network> network;
283294
if (fuzzed_data_provider.ConsumeBool()) {
284295
network = fuzzed_data_provider.PickValueInArray(ALL_NETWORKS);
285296
}
286-
(void)const_addr_man.GetAddr(
287-
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
288-
/*max_pct=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
289-
network,
290-
/*filtered=*/fuzzed_data_provider.ConsumeBool());
297+
auto max_addresses = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
298+
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
299+
auto filtered = fuzzed_data_provider.ConsumeBool();
300+
(void)const_addr_man.GetAddr(max_addresses, max_pct, network, filtered);
291301
(void)const_addr_man.Select(fuzzed_data_provider.ConsumeBool(), network);
292302
std::optional<bool> in_new;
293303
if (fuzzed_data_provider.ConsumeBool()) {

‎src/test/fuzz/banman.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,19 @@ FUZZ_TARGET(banman, .init = initialize_banman)
7676
} else {
7777
contains_invalid = true;
7878
}
79-
ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
79+
auto ban_time_offset = ConsumeBanTimeOffset(fuzzed_data_provider);
80+
auto since_unix_epoch = fuzzed_data_provider.ConsumeBool();
81+
ban_man.Ban(net_addr, ban_time_offset, since_unix_epoch);
8082
},
8183
[&] {
8284
CSubNet subnet{ConsumeSubNet(fuzzed_data_provider)};
8385
subnet = LookupSubNet(subnet.ToString());
8486
if (!subnet.IsValid()) {
8587
contains_invalid = true;
8688
}
87-
ban_man.Ban(subnet, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
89+
auto ban_time_offset = ConsumeBanTimeOffset(fuzzed_data_provider);
90+
auto since_unix_epoch = fuzzed_data_provider.ConsumeBool();
91+
ban_man.Ban(subnet, ban_time_offset, since_unix_epoch);
8892
},
8993
[&] {
9094
ban_man.ClearBanned();

‎src/test/fuzz/buffered_file.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ FUZZ_TARGET(buffered_file)
2525
ConsumeRandomLengthByteVector<std::byte>(fuzzed_data_provider),
2626
};
2727
try {
28-
opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096));
28+
auto n_buf_size = fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096);
29+
auto n_rewind_in = fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096);
30+
opt_buffered_file.emplace(fuzzed_file, n_buf_size, n_rewind_in);
2931
} catch (const std::ios_base::failure&) {
3032
}
3133
if (opt_buffered_file && !fuzzed_file.IsNull()) {

‎src/test/fuzz/connman.cpp

+7-9
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,15 @@ FUZZ_TARGET(connman, .init = initialize_connman)
8585
(void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral<NodeId>(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); });
8686
},
8787
[&] {
88-
(void)connman.GetAddresses(
89-
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
90-
/*max_pct=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
91-
/*network=*/std::nullopt,
92-
/*filtered=*/fuzzed_data_provider.ConsumeBool());
88+
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
89+
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
90+
auto filtered = fuzzed_data_provider.ConsumeBool();
91+
(void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
9392
},
9493
[&] {
95-
(void)connman.GetAddresses(
96-
/*requestor=*/random_node,
97-
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
98-
/*max_pct=*/fuzzed_data_provider.ConsumeIntegral<size_t>());
94+
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
95+
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
96+
(void)connman.GetAddresses(/*requestor=*/random_node, max_addresses, max_pct);
9997
},
10098
[&] {
10199
(void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral<uint64_t>());

‎src/test/fuzz/crypto.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ FUZZ_TARGET(crypto)
2222
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
2323
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
2424
if (data.empty()) {
25-
data.resize(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096), fuzzed_data_provider.ConsumeIntegral<uint8_t>());
25+
auto new_size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096);
26+
auto x = fuzzed_data_provider.ConsumeIntegral<uint8_t>();
27+
data.resize(new_size, x);
2628
}
2729

2830
CHash160 hash160;
@@ -44,7 +46,9 @@ FUZZ_TARGET(crypto)
4446
if (fuzzed_data_provider.ConsumeBool()) {
4547
data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
4648
if (data.empty()) {
47-
data.resize(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096), fuzzed_data_provider.ConsumeIntegral<uint8_t>());
49+
auto new_size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096);
50+
auto x = fuzzed_data_provider.ConsumeIntegral<uint8_t>();
51+
data.resize(new_size, x);
4852
}
4953
}
5054

‎src/test/fuzz/crypto_chacha20.cpp

+4-5
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,10 @@ FUZZ_TARGET(crypto_chacha20)
2828
chacha20.SetKey(key);
2929
},
3030
[&] {
31-
chacha20.Seek(
32-
{
33-
fuzzed_data_provider.ConsumeIntegral<uint32_t>(),
34-
fuzzed_data_provider.ConsumeIntegral<uint64_t>()
35-
}, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
31+
ChaCha20::Nonce96 nonce{
32+
fuzzed_data_provider.ConsumeIntegral<uint32_t>(),
33+
fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
34+
chacha20.Seek(nonce, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
3635
},
3736
[&] {
3837
std::vector<uint8_t> output(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096));

‎src/test/fuzz/cuckoocache.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ FUZZ_TARGET(cuckoocache)
4141
if (fuzzed_data_provider.ConsumeBool()) {
4242
cuckoo_cache.insert(fuzzed_data_provider.ConsumeBool());
4343
} else {
44-
cuckoo_cache.contains(fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool());
44+
auto e = fuzzed_data_provider.ConsumeBool();
45+
auto erase = fuzzed_data_provider.ConsumeBool();
46+
cuckoo_cache.contains(e, erase);
4547
}
4648
}
4749
fuzzed_data_provider_ptr = nullptr;

‎src/test/fuzz/message.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ FUZZ_TARGET(message, .init = initialize_message)
3939
}
4040
{
4141
(void)MessageHash(random_message);
42-
(void)MessageVerify(fuzzed_data_provider.ConsumeRandomLengthString(1024), fuzzed_data_provider.ConsumeRandomLengthString(1024), random_message);
42+
auto address = fuzzed_data_provider.ConsumeRandomLengthString(1024);
43+
auto signature = fuzzed_data_provider.ConsumeRandomLengthString(1024);
44+
(void)MessageVerify(address, signature, random_message);
4345
(void)SigningResultString(fuzzed_data_provider.PickValueInArray({SigningResult::OK, SigningResult::PRIVATE_KEY_NOT_AVAILABLE, SigningResult::SIGNING_FAILED}));
4446
}
4547
}

‎src/test/fuzz/policy_estimator.cpp

+11-2
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
8383
});
8484
(void)block_policy_estimator.estimateFee(fuzzed_data_provider.ConsumeIntegral<int>());
8585
EstimationResult result;
86-
(void)block_policy_estimator.estimateRawFee(fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeFloatingPoint<double>(), fuzzed_data_provider.PickValueInArray(ALL_FEE_ESTIMATE_HORIZONS), fuzzed_data_provider.ConsumeBool() ? &result : nullptr);
86+
auto conf_target = fuzzed_data_provider.ConsumeIntegral<int>();
87+
auto success_threshold = fuzzed_data_provider.ConsumeFloatingPoint<double>();
88+
auto horizon = fuzzed_data_provider.PickValueInArray(ALL_FEE_ESTIMATE_HORIZONS);
89+
auto* result_ptr = fuzzed_data_provider.ConsumeBool() ? &result : nullptr;
90+
(void)block_policy_estimator.estimateRawFee(conf_target, success_threshold, horizon, result_ptr);
91+
8792
FeeCalculation fee_calculation;
88-
(void)block_policy_estimator.estimateSmartFee(fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeBool() ? &fee_calculation : nullptr, fuzzed_data_provider.ConsumeBool());
93+
conf_target = fuzzed_data_provider.ConsumeIntegral<int>();
94+
auto* fee_calc_ptr = fuzzed_data_provider.ConsumeBool() ? &fee_calculation : nullptr;
95+
auto conservative = fuzzed_data_provider.ConsumeBool();
96+
(void)block_policy_estimator.estimateSmartFee(conf_target, fee_calc_ptr, conservative);
97+
8998
(void)block_policy_estimator.HighestTargetTracked(fuzzed_data_provider.PickValueInArray(ALL_FEE_ESTIMATE_HORIZONS));
9099
}
91100
{

‎src/test/fuzz/prevector.cpp

+21-12
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,20 @@ FUZZ_TARGET(prevector)
212212
LIMITED_WHILE(prov.remaining_bytes(), 3000)
213213
{
214214
switch (prov.ConsumeIntegralInRange<int>(0, 13 + 3 * (test.size() > 0))) {
215-
case 0:
216-
test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), prov.ConsumeIntegral<int>());
217-
break;
215+
case 0: {
216+
auto position = prov.ConsumeIntegralInRange<size_t>(0, test.size());
217+
auto value = prov.ConsumeIntegral<int>();
218+
test.insert(position, value);
219+
} break;
218220
case 1:
219221
test.resize(std::max(0, std::min(30, (int)test.size() + prov.ConsumeIntegralInRange<int>(0, 4) - 2)));
220222
break;
221-
case 2:
222-
test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), 1 + prov.ConsumeBool(), prov.ConsumeIntegral<int>());
223-
break;
223+
case 2: {
224+
auto position = prov.ConsumeIntegralInRange<size_t>(0, test.size());
225+
auto count = 1 + prov.ConsumeBool();
226+
auto value = prov.ConsumeIntegral<int>();
227+
test.insert(position, count, value);
228+
} break;
224229
case 3: {
225230
int del = prov.ConsumeIntegralInRange<int>(0, test.size());
226231
int beg = prov.ConsumeIntegralInRange<int>(0, test.size() - del);
@@ -257,9 +262,11 @@ FUZZ_TARGET(prevector)
257262
case 9:
258263
test.clear();
259264
break;
260-
case 10:
261-
test.assign(prov.ConsumeIntegralInRange<size_t>(0, 32767), prov.ConsumeIntegral<int>());
262-
break;
265+
case 10: {
266+
auto n = prov.ConsumeIntegralInRange<size_t>(0, 32767);
267+
auto value = prov.ConsumeIntegral<int>();
268+
test.assign(n, value);
269+
} break;
263270
case 11:
264271
test.swap();
265272
break;
@@ -269,9 +276,11 @@ FUZZ_TARGET(prevector)
269276
case 13:
270277
test.move();
271278
break;
272-
case 14:
273-
test.update(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1), prov.ConsumeIntegral<int>());
274-
break;
279+
case 14: {
280+
auto pos = prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1);
281+
auto value = prov.ConsumeIntegral<int>();
282+
test.update(pos, value);
283+
} break;
275284
case 15:
276285
test.erase(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1));
277286
break;

‎src/test/fuzz/script_format.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,7 @@ FUZZ_TARGET(script_format, .init = initialize_script_format)
3030
(void)ScriptToAsmStr(script, /*fAttemptSighashDecode=*/fuzzed_data_provider.ConsumeBool());
3131

3232
UniValue o1(UniValue::VOBJ);
33-
ScriptToUniv(script, /*out=*/o1, /*include_hex=*/fuzzed_data_provider.ConsumeBool(), /*include_address=*/fuzzed_data_provider.ConsumeBool());
33+
auto include_hex = fuzzed_data_provider.ConsumeBool();
34+
auto include_address = fuzzed_data_provider.ConsumeBool();
35+
ScriptToUniv(script, /*out=*/o1, include_hex, include_address);
3436
}

‎src/test/fuzz/script_interpreter.cpp

+8-2
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,18 @@ FUZZ_TARGET(script_interpreter)
2525
const CTransaction tx_to{*mtx};
2626
const unsigned int in = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
2727
if (in < tx_to.vin.size()) {
28-
(void)SignatureHash(script_code, tx_to, in, fuzzed_data_provider.ConsumeIntegral<int>(), ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}), nullptr);
28+
auto n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
29+
auto amount = ConsumeMoney(fuzzed_data_provider);
30+
auto sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
31+
(void)SignatureHash(script_code, tx_to, in, n_hash_type, amount, sigversion, nullptr);
2932
const std::optional<CMutableTransaction> mtx_precomputed = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
3033
if (mtx_precomputed) {
3134
const CTransaction tx_precomputed{*mtx_precomputed};
3235
const PrecomputedTransactionData precomputed_transaction_data{tx_precomputed};
33-
(void)SignatureHash(script_code, tx_to, in, fuzzed_data_provider.ConsumeIntegral<int>(), ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}), &precomputed_transaction_data);
36+
n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
37+
amount = ConsumeMoney(fuzzed_data_provider);
38+
sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
39+
(void)SignatureHash(script_code, tx_to, in, n_hash_type, amount, sigversion, &precomputed_transaction_data);
3440
}
3541
}
3642
}

‎src/test/fuzz/script_sign.cpp

+7-2
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
111111
}
112112
if (n_in < script_tx_to.vin.size()) {
113113
SignatureData empty;
114-
(void)SignSignature(provider, ConsumeScript(fuzzed_data_provider), script_tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>(), empty);
114+
auto from_pub_key = ConsumeScript(fuzzed_data_provider);
115+
auto amount = ConsumeMoney(fuzzed_data_provider);
116+
auto n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
117+
(void)SignSignature(provider, from_pub_key, script_tx_to, n_in, amount, n_hash_type, empty);
115118
MutableTransactionSignatureCreator signature_creator{tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>()};
116119
std::vector<unsigned char> vch_sig;
117120
CKeyID address;
@@ -122,7 +125,9 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
122125
} else {
123126
address = CKeyID{ConsumeUInt160(fuzzed_data_provider)};
124127
}
125-
(void)signature_creator.CreateSig(provider, vch_sig, address, ConsumeScript(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}));
128+
auto script_code = ConsumeScript(fuzzed_data_provider);
129+
auto sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
130+
(void)signature_creator.CreateSig(provider, vch_sig, address, script_code, sigversion);
126131
}
127132
std::map<COutPoint, Coin> coins{ConsumeCoins(fuzzed_data_provider)};
128133
std::map<int, bilingual_str> input_errors;

‎src/test/fuzz/socks5.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ FUZZ_TARGET(socks5, .init = initialize_socks5)
4141
FuzzedSock fuzzed_sock = ConsumeSock(fuzzed_data_provider);
4242
// This Socks5(...) fuzzing harness would have caught CVE-2017-18350 within
4343
// a few seconds of fuzzing.
44-
(void)Socks5(fuzzed_data_provider.ConsumeRandomLengthString(512),
45-
fuzzed_data_provider.ConsumeIntegral<uint16_t>(),
46-
fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr,
47-
fuzzed_sock);
44+
auto str_dest = fuzzed_data_provider.ConsumeRandomLengthString(512);
45+
auto port = fuzzed_data_provider.ConsumeIntegral<uint16_t>();
46+
auto* auth = fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr;
47+
(void)Socks5(str_dest, port, auth, fuzzed_sock);
4848
}

0 commit comments

Comments
 (0)