From 6408b24517f3418e2a408071b4c2ce26571f3167 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 8 Feb 2021 13:34:40 +0100 Subject: [PATCH 1/2] fuzz: move init code to the CAddrManDeterministic constructor Move the addrman init code from the test case to a newly added `CAddrManDeterministic` constructor. This way it can be reused by other tests. --- src/test/fuzz/addrman.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 344d1dde8e..94369fa5a0 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -25,10 +25,15 @@ void initialize_addrman() class CAddrManDeterministic : public CAddrMan { public: - void MakeDeterministic(const uint256& random_seed) + explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider) { - WITH_LOCK(cs, insecure_rand = FastRandomContext{random_seed}); - Clear(); + WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); + if (fuzzed_data_provider.ConsumeBool()) { + m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); + if (!SanityCheckASMap(m_asmap)) { + m_asmap.clear(); + } + } } }; @@ -36,14 +41,7 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - CAddrManDeterministic addr_man; - addr_man.MakeDeterministic(ConsumeUInt256(fuzzed_data_provider)); - if (fuzzed_data_provider.ConsumeBool()) { - addr_man.m_asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider); - if (!SanityCheckASMap(addr_man.m_asmap)) { - addr_man.m_asmap.clear(); - } - } + CAddrManDeterministic addr_man{fuzzed_data_provider}; if (fuzzed_data_provider.ConsumeBool()) { const std::vector serialized_data{ConsumeRandomLengthByteVector(fuzzed_data_provider)}; CDataStream ds(serialized_data, SER_DISK, INIT_PROTO_VERSION); From 87651795d8622d354f8e3c481eb868d9433b841c Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 8 Feb 2021 14:14:12 +0100 Subject: [PATCH 2/2] fuzz: check that ser+unser produces the same AddrMan --- src/addrman.h | 2 + src/test/fuzz/addrman.cpp | 204 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+) diff --git a/src/addrman.h b/src/addrman.h index 6347a24d55..5227857fa4 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -58,6 +58,7 @@ private: mutable int nRandomPos{-1}; friend class CAddrMan; + friend class CAddrManDeterministic; public: @@ -776,6 +777,7 @@ private: void RemoveInvalid() EXCLUSIVE_LOCKS_REQUIRED(cs); friend class CAddrManTest; + friend class CAddrManDeterministic; }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 94369fa5a0..4c29a8ee53 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -25,7 +26,10 @@ void initialize_addrman() class CAddrManDeterministic : public CAddrMan { public: + FuzzedDataProvider& m_fuzzed_data_provider; + explicit CAddrManDeterministic(FuzzedDataProvider& fuzzed_data_provider) + : m_fuzzed_data_provider(fuzzed_data_provider) { WITH_LOCK(cs, insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); if (fuzzed_data_provider.ConsumeBool()) { @@ -35,6 +39,188 @@ public: } } } + + /** + * Generate a random address. Always returns a valid address. + */ + CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(cs) + { + CNetAddr addr; + if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(m_fuzzed_data_provider); + } else { + // The networks [1..6] correspond to CNetAddr::BIP155Network (private). + static const std::map net_len_map = {{1, ADDR_IPV4_SIZE}, + {2, ADDR_IPV6_SIZE}, + {4, ADDR_TORV3_SIZE}, + {5, ADDR_I2P_SIZE}, + {6, ADDR_CJDNS_SIZE}}; + uint8_t net = insecure_rand.randrange(5) + 1; // [1..5] + if (net == 3) { + net = 6; + } + + CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); + + s << net; + s << insecure_rand.randbytes(net_len_map.at(net)); + + s >> addr; + } + + // Return a dummy IPv4 5.5.5.5 if we generated an invalid address. + if (!addr.IsValid()) { + in_addr v4_addr = {}; + v4_addr.s_addr = 0x05050505; + addr = CNetAddr{v4_addr}; + } + + return addr; + } + + /** + * Fill this addrman with lots of addresses from lots of sources. + */ + void Fill() + { + LOCK(cs); + + // Add some of the addresses directly to the "tried" table. + + // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% + const size_t n = m_fuzzed_data_provider.ConsumeIntegralInRange(0, 3); + + const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(10, 50); + CNetAddr prev_source; + // Use insecure_rand inside the loops instead of m_fuzzed_data_provider because when + // the latter is exhausted it just returns 0. + for (size_t i = 0; i < num_sources; ++i) { + const auto source = RandAddr(); + const size_t num_addresses = insecure_rand.randrange(500) + 1; // [1..500] + + for (size_t j = 0; j < num_addresses; ++j) { + const auto addr = CAddress{CService{RandAddr(), 8333}, NODE_NETWORK}; + const auto time_penalty = insecure_rand.randrange(100000001); +#if 1 + // 2.83 sec to fill. + if (n > 0 && mapInfo.size() % n == 0 && mapAddr.find(addr) == mapAddr.end()) { + // Add to the "tried" table (if the bucket slot is free). + const CAddrInfo dummy{addr, source}; + const int bucket = dummy.GetTriedBucket(nKey, m_asmap); + const int bucket_pos = dummy.GetBucketPosition(nKey, false, bucket); + if (vvTried[bucket][bucket_pos] == -1) { + int id; + CAddrInfo* addr_info = Create(addr, source, &id); + vvTried[bucket][bucket_pos] = id; + addr_info->fInTried = true; + ++nTried; + } + } else { + // Add to the "new" table. + Add_(addr, source, time_penalty); + } +#else + // 261.91 sec to fill. + Add_(addr, source, time_penalty); + if (n > 0 && mapInfo.size() % n == 0) { + Good_(addr, false, GetTime()); + } +#endif + // Add 10% of the addresses from more than one source. + if (insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { + Add_(addr, prev_source, time_penalty); + } + } + prev_source = source; + } + } + + /** + * Compare with another AddrMan. + * This compares: + * - the values in `mapInfo` (the keys aka ids are ignored) + * - vvNew entries refer to the same addresses + * - vvTried entries refer to the same addresses + */ + bool operator==(const CAddrManDeterministic& other) + { + LOCK2(cs, other.cs); + + if (mapInfo.size() != other.mapInfo.size() || nNew != other.nNew || + nTried != other.nTried) { + return false; + } + + // Check that all values in `mapInfo` are equal to all values in `other.mapInfo`. + // Keys may be different. + + using CAddrInfoHasher = std::function; + using CAddrInfoEq = std::function; + + CNetAddrHash netaddr_hasher; + + CAddrInfoHasher addrinfo_hasher = [&netaddr_hasher](const CAddrInfo& a) { + return netaddr_hasher(static_cast(a)) ^ netaddr_hasher(a.source) ^ + a.nLastSuccess ^ a.nAttempts ^ a.nRefCount ^ a.fInTried; + }; + + CAddrInfoEq addrinfo_eq = [](const CAddrInfo& lhs, const CAddrInfo& rhs) { + return static_cast(lhs) == static_cast(rhs) && + lhs.source == rhs.source && lhs.nLastSuccess == rhs.nLastSuccess && + lhs.nAttempts == rhs.nAttempts && lhs.nRefCount == rhs.nRefCount && + lhs.fInTried == rhs.fInTried; + }; + + using Addresses = std::unordered_set; + + const size_t num_addresses{mapInfo.size()}; + + Addresses addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : mapInfo) { + addresses.insert(addr); + } + + Addresses other_addresses{num_addresses, addrinfo_hasher, addrinfo_eq}; + for (const auto& [id, addr] : other.mapInfo) { + other_addresses.insert(addr); + } + + if (addresses != other_addresses) { + return false; + } + + auto IdsReferToSameAddress = [&](int id, int other_id) EXCLUSIVE_LOCKS_REQUIRED(cs, other.cs) { + if (id == -1 && other_id == -1) { + return true; + } + if ((id == -1 && other_id != -1) || (id != -1 && other_id == -1)) { + return false; + } + return mapInfo.at(id) == other.mapInfo.at(other_id); + }; + + // Check that `vvNew` contains the same addresses as `other.vvNew`. Notice - `vvNew[i][j]` + // contains just an id and the address is to be found in `mapInfo.at(id)`. The ids + // themselves may differ between `vvNew` and `other.vvNew`. + for (size_t i = 0; i < ADDRMAN_NEW_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvNew[i][j], other.vvNew[i][j])) { + return false; + } + } + } + + // Same for `vvTried`. + for (size_t i = 0; i < ADDRMAN_TRIED_BUCKET_COUNT; ++i) { + for (size_t j = 0; j < ADDRMAN_BUCKET_SIZE; ++j) { + if (!IdsReferToSameAddress(vvTried[i][j], other.vvTried[i][j])) { + return false; + } + } + } + + return true; + } }; FUZZ_TARGET_INIT(addrman, initialize_addrman) @@ -121,3 +307,21 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); data_stream << const_addr_man; } + +// Check that serialize followed by unserialize produces the same addrman. +FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + SetMockTime(ConsumeTime(fuzzed_data_provider)); + + CAddrManDeterministic addr_man1{fuzzed_data_provider}; + CAddrManDeterministic addr_man2{fuzzed_data_provider}; + addr_man2.m_asmap = addr_man1.m_asmap; + + CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); + + addr_man1.Fill(); + data_stream << addr_man1; + data_stream >> addr_man2; + assert(addr_man1 == addr_man2); +}