From 56303e382e26ac7096c09152c66894dc3bb4d1fd Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 21 Sep 2021 09:35:40 +0100 Subject: [PATCH 1/5] [fuzz] Create a FastRandomContext in addrman fuzz tests Don't reach inside the object-under-test to use its random context. --- src/test/fuzz/addrman.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index cfeab9dcdc..3dc9fdfaa4 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -50,7 +51,7 @@ public: /** * Generate a random address. Always returns a valid address. */ - CNetAddr RandAddr() EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) + CNetAddr RandAddr(FastRandomContext& fast_random_context) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) { CNetAddr addr; if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { @@ -62,7 +63,7 @@ public: {4, ADDR_TORV3_SIZE}, {5, ADDR_I2P_SIZE}, {6, ADDR_CJDNS_SIZE}}; - uint8_t net = m_impl->insecure_rand.randrange(5) + 1; // [1..5] + uint8_t net = fast_random_context.randrange(5) + 1; // [1..5] if (net == 3) { net = 6; } @@ -70,7 +71,7 @@ public: CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); s << net; - s << m_impl->insecure_rand.randbytes(net_len_map.at(net)); + s << fast_random_context.randbytes(net_len_map.at(net)); s >> addr; } @@ -99,15 +100,17 @@ public: const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(1, 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. + // Generate a FastRandomContext seed to use inside the loops instead of + // m_fuzzed_data_provider. When m_fuzzed_data_provider is exhausted it + // just returns 0. + FastRandomContext fast_random_context{ConsumeUInt256(m_fuzzed_data_provider)}; for (size_t i = 0; i < num_sources; ++i) { - const auto source = RandAddr(); - const size_t num_addresses = m_impl->insecure_rand.randrange(500) + 1; // [1..500] + const auto source = RandAddr(fast_random_context); + const size_t num_addresses = fast_random_context.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 = m_impl->insecure_rand.randrange(100000001); + const auto addr = CAddress{CService{RandAddr(fast_random_context), 8333}, NODE_NETWORK}; + const auto time_penalty = fast_random_context.randrange(100000001); m_impl->Add_(addr, source, time_penalty); if (n > 0 && m_impl->mapInfo.size() % n == 0) { @@ -115,8 +118,8 @@ public: } // Add 10% of the addresses from more than one source. - if (m_impl->insecure_rand.randrange(10) == 0 && prev_source.IsValid()) { - m_impl->Add_({addr}, prev_source, time_penalty); + if (fast_random_context.randrange(10) == 0 && prev_source.IsValid()) { + m_impl->Add_(addr, prev_source, time_penalty); } } prev_source = source; From 491975c596ebce93ae8de192c9ef171f002fac7c Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 21 Sep 2021 09:35:40 +0100 Subject: [PATCH 2/5] [fuzz] Pass FuzzedDataProvider& into Fill() in addrman fuzz tests Use a (reference) parameter instead of a data member of CAddrManDeterministic. This will allow us to make Fill() a free function in a later commit. Also remove CAddrManDeterministic.m_fuzzed_data_provider since it's no longer used. --- src/test/fuzz/addrman.cpp | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 3dc9fdfaa4..d797b3dd7a 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -39,11 +39,8 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman) class AddrManDeterministic : public AddrMan { public: - FuzzedDataProvider& m_fuzzed_data_provider; - explicit AddrManDeterministic(std::vector asmap, FuzzedDataProvider& fuzzed_data_provider) : AddrMan(std::move(asmap), /* deterministic */ true, /* consistency_check_ratio */ 0) - , m_fuzzed_data_provider(fuzzed_data_provider) { WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } @@ -51,11 +48,12 @@ public: /** * Generate a random address. Always returns a valid address. */ - CNetAddr RandAddr(FastRandomContext& fast_random_context) EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) + CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context) + EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) { CNetAddr addr; - if (m_fuzzed_data_provider.remaining_bytes() > 1 && m_fuzzed_data_provider.ConsumeBool()) { - addr = ConsumeNetAddr(m_fuzzed_data_provider); + if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(fuzzed_data_provider); } else { // The networks [1..6] correspond to CNetAddr::BIP155Network (private). static const std::map net_len_map = {{1, ADDR_IPV4_SIZE}, @@ -89,27 +87,27 @@ public: /** * Fill this addrman with lots of addresses from lots of sources. */ - void Fill() + void Fill(FuzzedDataProvider& fuzzed_data_provider) { LOCK(m_impl->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 n = fuzzed_data_provider.ConsumeIntegralInRange(0, 3); - const size_t num_sources = m_fuzzed_data_provider.ConsumeIntegralInRange(1, 50); + const size_t num_sources = fuzzed_data_provider.ConsumeIntegralInRange(1, 50); CNetAddr prev_source; // Generate a FastRandomContext seed to use inside the loops instead of - // m_fuzzed_data_provider. When m_fuzzed_data_provider is exhausted it + // fuzzed_data_provider. When fuzzed_data_provider is exhausted it // just returns 0. - FastRandomContext fast_random_context{ConsumeUInt256(m_fuzzed_data_provider)}; + FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; for (size_t i = 0; i < num_sources; ++i) { - const auto source = RandAddr(fast_random_context); + const auto source = RandAddr(fuzzed_data_provider, fast_random_context); const size_t num_addresses = fast_random_context.randrange(500) + 1; // [1..500] for (size_t j = 0; j < num_addresses; ++j) { - const auto addr = CAddress{CService{RandAddr(fast_random_context), 8333}, NODE_NETWORK}; + const auto addr = CAddress{CService{RandAddr(fuzzed_data_provider, fast_random_context), 8333}, NODE_NETWORK}; const auto time_penalty = fast_random_context.randrange(100000001); m_impl->Add_(addr, source, time_penalty); @@ -310,7 +308,7 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); - addr_man1.Fill(); + addr_man1.Fill(fuzzed_data_provider); data_stream << addr_man1; data_stream >> addr_man2; assert(addr_man1 == addr_man2); From 90ad8ad61a38dbb1f247a5f3d5f649a856d9938a Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 21 Sep 2021 10:48:32 +0100 Subject: [PATCH 3/5] [fuzz] Make RandAddr() a free function in fuzz/addrman.cpp It doesn't require access to CAddrManDeterministic --- src/test/fuzz/addrman.cpp | 77 +++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index d797b3dd7a..7b88e9b6c2 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -36,6 +36,44 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_addrman) } } +/** + * Generate a random address. Always returns a valid address. + */ +CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context) +{ + CNetAddr addr; + if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { + addr = ConsumeNetAddr(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 = fast_random_context.randrange(5) + 1; // [1..5] + if (net == 3) { + net = 6; + } + + CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); + + s << net; + s << fast_random_context.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; +} + class AddrManDeterministic : public AddrMan { public: @@ -45,45 +83,6 @@ public: WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } - /** - * Generate a random address. Always returns a valid address. - */ - CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& fast_random_context) - EXCLUSIVE_LOCKS_REQUIRED(m_impl->cs) - { - CNetAddr addr; - if (fuzzed_data_provider.remaining_bytes() > 1 && fuzzed_data_provider.ConsumeBool()) { - addr = ConsumeNetAddr(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 = fast_random_context.randrange(5) + 1; // [1..5] - if (net == 3) { - net = 6; - } - - CDataStream s(SER_NETWORK, PROTOCOL_VERSION | ADDRV2_FORMAT); - - s << net; - s << fast_random_context.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. */ From 640476eb0e17fd4c64d4840ceab229642f1d79d9 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 21 Sep 2021 10:48:32 +0100 Subject: [PATCH 4/5] [fuzz] Make Fill() a free function in fuzz/addrman.cpp Also rename it to FillAddrman and pass an addrman reference as an argument. Change FillAddrman to only use addrman's public interface methods. --- src/test/fuzz/addrman.cpp | 78 +++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 7b88e9b6c2..297656ac3d 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -74,6 +74,42 @@ CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& f return addr; } +/** Fill addrman with lots of addresses from lots of sources. */ +void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider) +{ + // Add some of the addresses directly to the "tried" table. + + // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% + const size_t n = fuzzed_data_provider.ConsumeIntegralInRange(0, 3); + + const size_t num_sources = fuzzed_data_provider.ConsumeIntegralInRange(1, 50); + CNetAddr prev_source; + // Generate a FastRandomContext seed to use inside the loops instead of + // fuzzed_data_provider. When fuzzed_data_provider is exhausted it + // just returns 0. + FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; + for (size_t i = 0; i < num_sources; ++i) { + const auto source = RandAddr(fuzzed_data_provider, fast_random_context); + const size_t num_addresses = fast_random_context.randrange(500) + 1; // [1..500] + + for (size_t j = 0; j < num_addresses; ++j) { + const auto addr = CAddress{CService{RandAddr(fuzzed_data_provider, fast_random_context), 8333}, NODE_NETWORK}; + const auto time_penalty = fast_random_context.randrange(100000001); + addrman.Add({addr}, source, time_penalty); + + if (n > 0 && addrman.size() % n == 0) { + addrman.Good(addr, GetTime()); + } + + // Add 10% of the addresses from more than one source. + if (fast_random_context.randrange(10) == 0 && prev_source.IsValid()) { + addrman.Add({addr}, prev_source, time_penalty); + } + } + prev_source = source; + } +} + class AddrManDeterministic : public AddrMan { public: @@ -83,46 +119,6 @@ public: WITH_LOCK(m_impl->cs, m_impl->insecure_rand = FastRandomContext{ConsumeUInt256(fuzzed_data_provider)}); } - /** - * Fill this addrman with lots of addresses from lots of sources. - */ - void Fill(FuzzedDataProvider& fuzzed_data_provider) - { - LOCK(m_impl->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 = fuzzed_data_provider.ConsumeIntegralInRange(0, 3); - - const size_t num_sources = fuzzed_data_provider.ConsumeIntegralInRange(1, 50); - CNetAddr prev_source; - // Generate a FastRandomContext seed to use inside the loops instead of - // fuzzed_data_provider. When fuzzed_data_provider is exhausted it - // just returns 0. - FastRandomContext fast_random_context{ConsumeUInt256(fuzzed_data_provider)}; - for (size_t i = 0; i < num_sources; ++i) { - const auto source = RandAddr(fuzzed_data_provider, fast_random_context); - const size_t num_addresses = fast_random_context.randrange(500) + 1; // [1..500] - - for (size_t j = 0; j < num_addresses; ++j) { - const auto addr = CAddress{CService{RandAddr(fuzzed_data_provider, fast_random_context), 8333}, NODE_NETWORK}; - const auto time_penalty = fast_random_context.randrange(100000001); - m_impl->Add_(addr, source, time_penalty); - - if (n > 0 && m_impl->mapInfo.size() % n == 0) { - m_impl->Good_(addr, false, GetTime()); - } - - // Add 10% of the addresses from more than one source. - if (fast_random_context.randrange(10) == 0 && prev_source.IsValid()) { - m_impl->Add_(addr, prev_source, time_penalty); - } - } - prev_source = source; - } - } - /** * Compare with another AddrMan. * This compares: @@ -307,7 +303,7 @@ FUZZ_TARGET_INIT(addrman_serdeser, initialize_addrman) CDataStream data_stream(SER_NETWORK, PROTOCOL_VERSION); - addr_man1.Fill(fuzzed_data_provider); + FillAddrman(addr_man1, fuzzed_data_provider); data_stream << addr_man1; data_stream >> addr_man2; assert(addr_man1 == addr_man2); From 44452110f0fa7cc1bcb941a3c7b5db4b492a7b9c Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 21 Sep 2021 11:01:02 +0100 Subject: [PATCH 5/5] [fuzz] Update comment in FillAddrman() Suggested here: https://github.com/bitcoin/bitcoin/pull/22974#discussion_r711119626 --- src/test/fuzz/addrman.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 297656ac3d..8df3707fc9 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -77,8 +77,7 @@ CNetAddr RandAddr(FuzzedDataProvider& fuzzed_data_provider, FastRandomContext& f /** Fill addrman with lots of addresses from lots of sources. */ void FillAddrman(AddrMan& addrman, FuzzedDataProvider& fuzzed_data_provider) { - // Add some of the addresses directly to the "tried" table. - + // Add a fraction of the addresses to the "tried" table. // 0, 1, 2, 3 corresponding to 0%, 100%, 50%, 33% const size_t n = fuzzed_data_provider.ConsumeIntegralInRange(0, 3);