Merge bitcoin/bitcoin#27935: fuzz: call lookup functions before calling Ban

fca0a8938e ci: remove "--exclude banman" for fuzzing in mac (brunoerg)
f9b286353f fuzz: call lookup functions before calling `Ban` (brunoerg)

Pull request description:

  Fixes #27924

  To not have any discrepancy, it's required to call lookup functions before calling `Ban`. If we don't do it, the assertion `assert(banmap == banmap_read);` may fail because `BanMapFromJson` will call `LookupSubNet` and cause the discrepancy between the banned and the loaded one. It happens especially in MacOS (#27924).

  Also, calling lookup functions before banning is what RPC `setban` does.

ACKs for top commit:
  maflcko:
    lgtm ACK fca0a8938e
  dergoegge:
    ACK fca0a8938e

Tree-SHA512: a3d635088a556df4507e65542157f10b41d4f87dce42927b58c3b812f262f4544b6b57f3384eef1097ffdd7c32b8dd1556aae201254960cbfbf48d45551200f7
This commit is contained in:
fanquake 2023-11-13 10:52:50 +00:00
commit e862bceb17
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 18 additions and 6 deletions

View file

@ -15,4 +15,3 @@ export NO_DEPENDS=1
export OSX_SDK=""
export CCACHE_MAXSIZE=400M
export RUN_FUZZ_TESTS=true
export FUZZ_TESTS_CONFIG="--exclude banman" # https://github.com/bitcoin/bitcoin/issues/27924

View file

@ -63,17 +63,28 @@ FUZZ_TARGET(banman, .init = initialize_banman)
// The complexity is O(N^2), where N is the input size, because each call
// might call DumpBanlist (or other methods that are at least linear
// complexity of the input size).
bool contains_invalid{false};
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300)
{
CallOneOf(
fuzzed_data_provider,
[&] {
ban_man.Ban(ConsumeNetAddr(fuzzed_data_provider),
ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
CNetAddr net_addr{ConsumeNetAddr(fuzzed_data_provider)};
const std::optional<CNetAddr>& addr{LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false)};
if (addr.has_value() && addr->IsValid()) {
net_addr = *addr;
} else {
contains_invalid = true;
}
ban_man.Ban(net_addr, ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
},
[&] {
ban_man.Ban(ConsumeSubNet(fuzzed_data_provider),
ConsumeBanTimeOffset(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool());
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());
},
[&] {
ban_man.ClearBanned();
@ -109,7 +120,9 @@ FUZZ_TARGET(banman, .init = initialize_banman)
BanMan ban_man_read{banlist_file, /*client_interface=*/nullptr, /*default_ban_time=*/0};
banmap_t banmap_read;
ban_man_read.GetBanned(banmap_read);
assert(banmap == banmap_read);
if (!contains_invalid) {
assert(banmap == banmap_read);
}
}
}
fs::remove(fs::PathToString(banlist_file + ".json"));