Skip to content

Commit

Permalink
fuzz: make FuzzedDataProvider usage deterministic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
martinus committed Dec 9, 2023
1 parent 3e69125 commit 01960c5
Show file tree
Hide file tree
Showing 18 changed files with 129 additions and 66 deletions.
30 changes: 20 additions & 10 deletions src/test/fuzz/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,31 +263,41 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
addresses.push_back(ConsumeAddress(fuzzed_data_provider));
}
addr_man.Add(addresses, ConsumeNetAddr(fuzzed_data_provider), std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)});
auto net_addr = ConsumeNetAddr(fuzzed_data_provider);
auto time_penalty = std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)};
addr_man.Add(addresses, net_addr, time_penalty);
},
[&] {
addr_man.Good(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
auto addr = ConsumeService(fuzzed_data_provider);
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
addr_man.Good(addr, time);
},
[&] {
addr_man.Attempt(ConsumeService(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
auto addr = ConsumeService(fuzzed_data_provider);
auto count_failure = fuzzed_data_provider.ConsumeBool();
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
addr_man.Attempt(addr, count_failure, time);
},
[&] {
addr_man.Connected(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
auto addr = ConsumeService(fuzzed_data_provider);
auto time = NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}};
addr_man.Connected(addr, time);
},
[&] {
addr_man.SetServices(ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
auto addr = ConsumeService(fuzzed_data_provider);
auto n_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
addr_man.SetServices(addr, n_services);
});
}
const AddrMan& const_addr_man{addr_man};
std::optional<Network> network;
if (fuzzed_data_provider.ConsumeBool()) {
network = fuzzed_data_provider.PickValueInArray(ALL_NETWORKS);
}
(void)const_addr_man.GetAddr(
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
/*max_pct=*/fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096),
network,
/*filtered=*/fuzzed_data_provider.ConsumeBool());
auto max_addresses = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
auto max_pct = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096);
auto filtered = fuzzed_data_provider.ConsumeBool();
(void)const_addr_man.GetAddr(max_addresses, max_pct, network, filtered);
(void)const_addr_man.Select(fuzzed_data_provider.ConsumeBool(), network);
std::optional<bool> in_new;
if (fuzzed_data_provider.ConsumeBool()) {
Expand Down
8 changes: 6 additions & 2 deletions src/test/fuzz/banman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,19 @@ FUZZ_TARGET(banman, .init = initialize_banman)
} else {
contains_invalid = true;
}
ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
auto ban_time_offset = ConsumeBanTimeOffset(fuzzed_data_provider);
auto since_unix_epoch = fuzzed_data_provider.ConsumeBool();
ban_man.Ban(net_addr, ban_time_offset, since_unix_epoch);
},
[&] {
CSubNet subnet{ConsumeSubNet(fuzzed_data_provider)};
subnet = LookupSubNet(subnet.ToString());
if (!subnet.IsValid()) {
contains_invalid = true;
}
ban_man.Ban(subnet, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
auto ban_time_offset = ConsumeBanTimeOffset(fuzzed_data_provider);
auto since_unix_epoch = fuzzed_data_provider.ConsumeBool();
ban_man.Ban(subnet, ban_time_offset, since_unix_epoch);
},
[&] {
ban_man.ClearBanned();
Expand Down
4 changes: 3 additions & 1 deletion src/test/fuzz/buffered_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ FUZZ_TARGET(buffered_file)
ConsumeRandomLengthByteVector<std::byte>(fuzzed_data_provider),
};
try {
opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096));
auto n_buf_size = fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096);
auto n_rewind_in = fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096);
opt_buffered_file.emplace(fuzzed_file, n_buf_size, n_rewind_in);
} catch (const std::ios_base::failure&) {
}
if (opt_buffered_file && !fuzzed_file.IsNull()) {
Expand Down
16 changes: 7 additions & 9 deletions src/test/fuzz/connman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,15 @@ FUZZ_TARGET(connman, .init = initialize_connman)
(void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral<NodeId>(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); });
},
[&] {
(void)connman.GetAddresses(
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
/*max_pct=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
/*network=*/std::nullopt,
/*filtered=*/fuzzed_data_provider.ConsumeBool());
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto filtered = fuzzed_data_provider.ConsumeBool();
(void)connman.GetAddresses(max_addresses, max_pct, /*network=*/std::nullopt, filtered);
},
[&] {
(void)connman.GetAddresses(
/*requestor=*/random_node,
/*max_addresses=*/fuzzed_data_provider.ConsumeIntegral<size_t>(),
/*max_pct=*/fuzzed_data_provider.ConsumeIntegral<size_t>());
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
auto max_pct = fuzzed_data_provider.ConsumeIntegral<size_t>();
(void)connman.GetAddresses(/*requestor=*/random_node, max_addresses, max_pct);
},
[&] {
(void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
Expand Down
8 changes: 6 additions & 2 deletions src/test/fuzz/crypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ FUZZ_TARGET(crypto)
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
std::vector<uint8_t> data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
data.resize(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096), fuzzed_data_provider.ConsumeIntegral<uint8_t>());
auto new_size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096);
auto x = fuzzed_data_provider.ConsumeIntegral<uint8_t>();
data.resize(new_size, x);
}

CHash160 hash160;
Expand All @@ -44,7 +46,9 @@ FUZZ_TARGET(crypto)
if (fuzzed_data_provider.ConsumeBool()) {
data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
if (data.empty()) {
data.resize(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096), fuzzed_data_provider.ConsumeIntegral<uint8_t>());
auto new_size = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 4096);
auto x = fuzzed_data_provider.ConsumeIntegral<uint8_t>();
data.resize(new_size, x);
}
}

Expand Down
9 changes: 4 additions & 5 deletions src/test/fuzz/crypto_chacha20.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@ FUZZ_TARGET(crypto_chacha20)
chacha20.SetKey(key);
},
[&] {
chacha20.Seek(
{
fuzzed_data_provider.ConsumeIntegral<uint32_t>(),
fuzzed_data_provider.ConsumeIntegral<uint64_t>()
}, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
ChaCha20::Nonce96 nonce{
fuzzed_data_provider.ConsumeIntegral<uint32_t>(),
fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
chacha20.Seek(nonce, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
},
[&] {
std::vector<uint8_t> output(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096));
Expand Down
4 changes: 3 additions & 1 deletion src/test/fuzz/cuckoocache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ FUZZ_TARGET(cuckoocache)
if (fuzzed_data_provider.ConsumeBool()) {
cuckoo_cache.insert(fuzzed_data_provider.ConsumeBool());
} else {
cuckoo_cache.contains(fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool());
auto e = fuzzed_data_provider.ConsumeBool();
auto erase = fuzzed_data_provider.ConsumeBool();
cuckoo_cache.contains(e, erase);
}
}
fuzzed_data_provider_ptr = nullptr;
Expand Down
4 changes: 3 additions & 1 deletion src/test/fuzz/message.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ FUZZ_TARGET(message, .init = initialize_message)
}
{
(void)MessageHash(random_message);
(void)MessageVerify(fuzzed_data_provider.ConsumeRandomLengthString(1024), fuzzed_data_provider.ConsumeRandomLengthString(1024), random_message);
auto address = fuzzed_data_provider.ConsumeRandomLengthString(1024);
auto signature = fuzzed_data_provider.ConsumeRandomLengthString(1024);
(void)MessageVerify(address, signature, random_message);
(void)SigningResultString(fuzzed_data_provider.PickValueInArray({SigningResult::OK, SigningResult::PRIVATE_KEY_NOT_AVAILABLE, SigningResult::SIGNING_FAILED}));
}
}
13 changes: 11 additions & 2 deletions src/test/fuzz/policy_estimator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
});
(void)block_policy_estimator.estimateFee(fuzzed_data_provider.ConsumeIntegral<int>());
EstimationResult result;
(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);
auto conf_target = fuzzed_data_provider.ConsumeIntegral<int>();
auto success_threshold = fuzzed_data_provider.ConsumeFloatingPoint<double>();
auto horizon = fuzzed_data_provider.PickValueInArray(ALL_FEE_ESTIMATE_HORIZONS);
auto* result_ptr = fuzzed_data_provider.ConsumeBool() ? &result : nullptr;
(void)block_policy_estimator.estimateRawFee(conf_target, success_threshold, horizon, result_ptr);

FeeCalculation fee_calculation;
(void)block_policy_estimator.estimateSmartFee(fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeBool() ? &fee_calculation : nullptr, fuzzed_data_provider.ConsumeBool());
conf_target = fuzzed_data_provider.ConsumeIntegral<int>();
auto* fee_calc_ptr = fuzzed_data_provider.ConsumeBool() ? &fee_calculation : nullptr;
auto conservative = fuzzed_data_provider.ConsumeBool();
(void)block_policy_estimator.estimateSmartFee(conf_target, fee_calc_ptr, conservative);

(void)block_policy_estimator.HighestTargetTracked(fuzzed_data_provider.PickValueInArray(ALL_FEE_ESTIMATE_HORIZONS));
}
{
Expand Down
33 changes: 21 additions & 12 deletions src/test/fuzz/prevector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,15 +212,20 @@ FUZZ_TARGET(prevector)
LIMITED_WHILE(prov.remaining_bytes(), 3000)
{
switch (prov.ConsumeIntegralInRange<int>(0, 13 + 3 * (test.size() > 0))) {
case 0:
test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), prov.ConsumeIntegral<int>());
break;
case 0: {
auto position = prov.ConsumeIntegralInRange<size_t>(0, test.size());
auto value = prov.ConsumeIntegral<int>();
test.insert(position, value);
} break;
case 1:
test.resize(std::max(0, std::min(30, (int)test.size() + prov.ConsumeIntegralInRange<int>(0, 4) - 2)));
break;
case 2:
test.insert(prov.ConsumeIntegralInRange<size_t>(0, test.size()), 1 + prov.ConsumeBool(), prov.ConsumeIntegral<int>());
break;
case 2: {
auto position = prov.ConsumeIntegralInRange<size_t>(0, test.size());
auto count = 1 + prov.ConsumeBool();
auto value = prov.ConsumeIntegral<int>();
test.insert(position, count, value);
} break;
case 3: {
int del = prov.ConsumeIntegralInRange<int>(0, test.size());
int beg = prov.ConsumeIntegralInRange<int>(0, test.size() - del);
Expand Down Expand Up @@ -257,9 +262,11 @@ FUZZ_TARGET(prevector)
case 9:
test.clear();
break;
case 10:
test.assign(prov.ConsumeIntegralInRange<size_t>(0, 32767), prov.ConsumeIntegral<int>());
break;
case 10: {
auto n = prov.ConsumeIntegralInRange<size_t>(0, 32767);
auto value = prov.ConsumeIntegral<int>();
test.assign(n, value);
} break;
case 11:
test.swap();
break;
Expand All @@ -269,9 +276,11 @@ FUZZ_TARGET(prevector)
case 13:
test.move();
break;
case 14:
test.update(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1), prov.ConsumeIntegral<int>());
break;
case 14: {
auto pos = prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1);
auto value = prov.ConsumeIntegral<int>();
test.update(pos, value);
} break;
case 15:
test.erase(prov.ConsumeIntegralInRange<size_t>(0, test.size() - 1));
break;
Expand Down
4 changes: 3 additions & 1 deletion src/test/fuzz/script_format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,7 @@ FUZZ_TARGET(script_format, .init = initialize_script_format)
(void)ScriptToAsmStr(script, /*fAttemptSighashDecode=*/fuzzed_data_provider.ConsumeBool());

UniValue o1(UniValue::VOBJ);
ScriptToUniv(script, /*out=*/o1, /*include_hex=*/fuzzed_data_provider.ConsumeBool(), /*include_address=*/fuzzed_data_provider.ConsumeBool());
auto include_hex = fuzzed_data_provider.ConsumeBool();
auto include_address = fuzzed_data_provider.ConsumeBool();
ScriptToUniv(script, /*out=*/o1, include_hex, include_address);
}
10 changes: 8 additions & 2 deletions src/test/fuzz/script_interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,18 @@ FUZZ_TARGET(script_interpreter)
const CTransaction tx_to{*mtx};
const unsigned int in = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
if (in < tx_to.vin.size()) {
(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);
auto n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
auto amount = ConsumeMoney(fuzzed_data_provider);
auto sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
(void)SignatureHash(script_code, tx_to, in, n_hash_type, amount, sigversion, nullptr);
const std::optional<CMutableTransaction> mtx_precomputed = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS);
if (mtx_precomputed) {
const CTransaction tx_precomputed{*mtx_precomputed};
const PrecomputedTransactionData precomputed_transaction_data{tx_precomputed};
(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);
n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
amount = ConsumeMoney(fuzzed_data_provider);
sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
(void)SignatureHash(script_code, tx_to, in, n_hash_type, amount, sigversion, &precomputed_transaction_data);
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/test/fuzz/script_sign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
}
if (n_in < script_tx_to.vin.size()) {
SignatureData empty;
(void)SignSignature(provider, ConsumeScript(fuzzed_data_provider), script_tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>(), empty);
auto from_pub_key = ConsumeScript(fuzzed_data_provider);
auto amount = ConsumeMoney(fuzzed_data_provider);
auto n_hash_type = fuzzed_data_provider.ConsumeIntegral<int>();
(void)SignSignature(provider, from_pub_key, script_tx_to, n_in, amount, n_hash_type, empty);
MutableTransactionSignatureCreator signature_creator{tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral<int>()};
std::vector<unsigned char> vch_sig;
CKeyID address;
Expand All @@ -122,7 +125,9 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
} else {
address = CKeyID{ConsumeUInt160(fuzzed_data_provider)};
}
(void)signature_creator.CreateSig(provider, vch_sig, address, ConsumeScript(fuzzed_data_provider), fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0}));
auto script_code = ConsumeScript(fuzzed_data_provider);
auto sigversion = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
(void)signature_creator.CreateSig(provider, vch_sig, address, script_code, sigversion);
}
std::map<COutPoint, Coin> coins{ConsumeCoins(fuzzed_data_provider)};
std::map<int, bilingual_str> input_errors;
Expand Down
8 changes: 4 additions & 4 deletions src/test/fuzz/socks5.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ FUZZ_TARGET(socks5, .init = initialize_socks5)
FuzzedSock fuzzed_sock = ConsumeSock(fuzzed_data_provider);
// This Socks5(...) fuzzing harness would have caught CVE-2017-18350 within
// a few seconds of fuzzing.
(void)Socks5(fuzzed_data_provider.ConsumeRandomLengthString(512),
fuzzed_data_provider.ConsumeIntegral<uint16_t>(),
fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr,
fuzzed_sock);
auto str_dest = fuzzed_data_provider.ConsumeRandomLengthString(512);
auto port = fuzzed_data_provider.ConsumeIntegral<uint16_t>();
auto* auth = fuzzed_data_provider.ConsumeBool() ? &proxy_credentials : nullptr;
(void)Socks5(str_dest, port, auth, fuzzed_sock);
}
Loading

0 comments on commit 01960c5

Please sign in to comment.