aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorfanquake <fanquake@gmail.com>2023-11-13 10:52:50 +0000
committerfanquake <fanquake@gmail.com>2023-11-13 10:57:01 +0000
commite862bceb1705e6d8c1b738301ed10ae1b4413bfd (patch)
tree04f0a061c4443be415cbc681cc74f2e19a119667
parentdd5f5713bccac3061676cf3814dcd4488ac557ed (diff)
parentfca0a8938e34cb4f6c400e1d1d0be02f027d80c5 (diff)
downloadbitcoin-e862bceb1705e6d8c1b738301ed10ae1b4413bfd.tar.xz
Merge bitcoin/bitcoin#27935: fuzz: call lookup functions before calling `Ban`
fca0a8938e34cb4f6c400e1d1d0be02f027d80c5 ci: remove "--exclude banman" for fuzzing in mac (brunoerg) f9b286353f79cdb5e55e2ff4ca47d73e14f9da48 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 fca0a8938e34cb4f6c400e1d1d0be02f027d80c5 dergoegge: ACK fca0a8938e34cb4f6c400e1d1d0be02f027d80c5 Tree-SHA512: a3d635088a556df4507e65542157f10b41d4f87dce42927b58c3b812f262f4544b6b57f3384eef1097ffdd7c32b8dd1556aae201254960cbfbf48d45551200f7
-rwxr-xr-xci/test/00_setup_env_mac_native.sh1
-rw-r--r--src/test/fuzz/banman.cpp23
2 files changed, 18 insertions, 6 deletions
diff --git a/ci/test/00_setup_env_mac_native.sh b/ci/test/00_setup_env_mac_native.sh
index c9f65bf397..439fba16ef 100755
--- a/ci/test/00_setup_env_mac_native.sh
+++ b/ci/test/00_setup_env_mac_native.sh
@@ -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
diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp
index 3882e0e547..4a040c56de 100644
--- a/src/test/fuzz/banman.cpp
+++ b/src/test/fuzz/banman.cpp
@@ -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"));