Skip to content

Commit

Permalink
Merge #29043: fuzz: make FuzzedDataProvider usage deterministic
Browse files Browse the repository at this point in the history 
01960c5 fuzz: make FuzzedDataProvider usage deterministic (Martin Leitner-Ankerl)

Pull request description:

  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](https://en.cppreference.com/w/cpp/language/eval_order), and a simple example shows that it can differ e.g. between clang++ and g++: https://godbolt.org/z/jooMezWWY

  When the evaluation order is not consistent, the same fuzzing/random input will produce different output, which is bad for coverage/reproducibility. This PR fixes all these cases I have found where unspecified evaluation order could be a problem.

  Finding these has been manual work; I grepped the sourcecode for these patterns, and looked at each usage individually. So there is a chance I missed some.

  * `fuzzed_data_provider`
  * `.Consume`
  * `>Consume`
  * `.rand`

  I first discovered this in #29013 (comment). Note that there is a possibility that due to this fix the evaluation order is now different in many cases than when the fuzzing corpus has been created. If that is the case, the fuzzing corpus will have worse coverage than before.

  Update: In list-initialization the order of evaluation is well defined, so e.g. usages in `initializer_list` or constructors that use `{...}` is ok.

ACKs for top commit:
  achow101:
    ACK 01960c5
  vasild:
    ACK 01960c5
  ismaelsadeeq:
    ACK 01960c5

Tree-SHA512: e56d087f6f4bf79c90b972a5f0c6908d1784b3cfbb8130b6b450d5ca7d116c5a791df506b869a23bce930b2a6977558e1fb5115bb4e061969cc40f568077a1ad
  • Loading branch information
@achow101
achow101 committed Sep 4, 2024
2 parents 8127654 + 01960c5 commit 3210d87
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,31 +250,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,19 @@ FUZZ_TARGET(banman, .init = initialize_banman)
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,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
View file
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
View file
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
View file
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,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 @@ -255,9 +260,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 @@ -267,9 +274,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
View file
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
View file
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
View file
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
View file
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 3210d87

Please sign in to comment.