Merge #20789: fuzz: Rework strong and weak net enum fuzzing

eeee43bc48ea7fbacd3c5e3f076f01f04744adb8 fuzz: Use ConsumeWeakEnum for ServiceFlags (MarcoFalke)
fa9949b91414ee0da376a322cee32ba4e3989d8c fuzz: Add ConsumeWeakEnum helper, Extract ALL_NET_PERMISSION_FLAGS (MarcoFalke)
faaef9434c19e3643322ee442c240c166af5adbd fuzz: [refactor] Extract ALL_CONNECTION_TYPES constant (MarcoFalke)
fa42da2d5424c0aeccfae4b49fde2bea330b63dc fuzz: Use ConsumeNode in process_message target (MarcoFalke)
fa121f058fdc5f09dd11678480f551246cb3c5e2 fuzz: Use ConsumeNode in process_messages target (MarcoFalke)

Pull request description:

  The fuzz tests have several problems:
  * The array passed to the fuzz engine to pick `net_permission_flags` is outdated
  * The process_message* targets has the service flags as well as connection type hardcoded, limiting potential coverage
  * The service flags deserialization from the fuzz engine doesn't allow for easy "exact matches". The fuzz engine has to explore a 64-bit space to hit an "exact match" (only one bit set)

  Fix all issues in the commits in this pull

ACKs for top commit:
  mzumsande:
    ACK eeee43bc48ea7fbacd3c5e3f076f01f04744adb8 after rebase.

Tree-SHA512: 1ad9520c7e708b7f4994ae8f77886ffca33d7c542756e2a3e07dbbbe59e360f9fcaccf2e2fb57d9bc731d4aeb4938fb1c5c546e9d2744b007af5626f5cb377fe

7 files changed, 62 insertions, 27 deletions

diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp
index 7621751077..fdf51d8558 100644
--- a/src/test/fuzz/connman.cpp
+++ b/src/test/fuzz/connman.cpp
@@ -128,7 +128,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
             connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool());
             break;
         case 26:
-            connman.SetServices(random_service, static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()));
+            connman.SetServices(random_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
             break;
         case 27:
             connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool());
diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp
index ec02cf94e9..3ca921b5cf 100644
--- a/src/test/fuzz/net.cpp
+++ b/src/test/fuzz/net.cpp
@@ -13,6 +13,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/net.h>
 #include <test/util/setup_common.h>
 
 #include <cstdint>
@@ -122,9 +123,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
     (void)node.GetCommonVersion();
     (void)node.RelayAddrsWithConn();
 
-    const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ?
-                                                        fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({NetPermissionFlags::PF_NONE, NetPermissionFlags::PF_BLOOMFILTER, NetPermissionFlags::PF_RELAY, NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL}) :
-                                                        static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
+    const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
     (void)node.HasPermission(net_permission_flags);
     (void)node.ConnectedThroughNetwork();
 }
diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp
index 3620e16d30..544a33047b 100644
--- a/src/test/fuzz/net_permissions.cpp
+++ b/src/test/fuzz/net_permissions.cpp
@@ -17,18 +17,7 @@ FUZZ_TARGET(net_permissions)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(32);
-    const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ? fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({
-                                                                                             NetPermissionFlags::PF_NONE,
-                                                                                             NetPermissionFlags::PF_BLOOMFILTER,
-                                                                                             NetPermissionFlags::PF_RELAY,
-                                                                                             NetPermissionFlags::PF_FORCERELAY,
-                                                                                             NetPermissionFlags::PF_NOBAN,
-                                                                                             NetPermissionFlags::PF_MEMPOOL,
-                                                                                             NetPermissionFlags::PF_ADDR,
-                                                                                             NetPermissionFlags::PF_ISIMPLICIT,
-                                                                                             NetPermissionFlags::PF_ALL,
-                                                                                         }) :
-                                                                                         static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
+    const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
 
     NetWhitebindPermissions net_whitebind_permissions;
     bilingual_str error_net_whitebind_permissions;
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index 97e2b04a7d..b6865c5347 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -13,6 +13,7 @@
 #include <streams.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
 #include <test/util/mining.h>
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
@@ -63,13 +64,15 @@ void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO
     }
     const bool jump_out_of_ibd{fuzzed_data_provider.ConsumeBool()};
     if (jump_out_of_ibd) chainstate.JumpOutOfIbd();
-    CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
-    CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY).release();
+    CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release();
     p2p_node.fSuccessfullyConnected = true;
     p2p_node.nVersion = PROTOCOL_VERSION;
     p2p_node.SetCommonVersion(PROTOCOL_VERSION);
     connman.AddTestNode(p2p_node);
     g_setup->m_node.peerman->InitializeNode(&p2p_node);
+
+    // fuzzed_data_provider is fully consumed after this call, don't use it
+    CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
     try {
         g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream,
                                                 GetTime<std::chrono::microseconds>(), std::atomic<bool>{false});
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index 0ff95ea1ae..fcfa623508 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -47,9 +47,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
 
     const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3);
     for (int i = 0; i < num_peers_to_add; ++i) {
-        const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
-        const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
-        peers.push_back(MakeUnique<CNode>(i, service_flags, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release());
+        peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release());
         CNode& p2p_node = *peers.back();
 
         p2p_node.fSuccessfullyConnected = true;
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
index ff79dfe5f3..94c691936e 100644
--- a/src/test/fuzz/util.h
+++ b/src/test/fuzz/util.h
@@ -22,6 +22,7 @@
 #include <streams.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
+#include <test/util/net.h>
 #include <test/util/setup_common.h>
 #include <txmempool.h>
 #include <uint256.h>
@@ -86,6 +87,14 @@ template <typename T>
     return obj;
 }
 
+template <typename WeakEnumType, size_t size>
+[[nodiscard]] WeakEnumType ConsumeWeakEnum(FuzzedDataProvider& fuzzed_data_provider, const WeakEnumType (&all_types)[size]) noexcept
+{
+    return fuzzed_data_provider.ConsumeBool() ?
+               fuzzed_data_provider.PickValueInArray<WeakEnumType>(all_types) :
+               WeakEnumType(fuzzed_data_provider.ConsumeIntegral<typename std::underlying_type<WeakEnumType>::type>());
+}
+
 [[nodiscard]] inline opcodetype ConsumeOpcodeType(FuzzedDataProvider& fuzzed_data_provider) noexcept
 {
     return static_cast<opcodetype>(fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, MAX_OPCODE));
@@ -283,23 +292,29 @@ inline CService ConsumeService(FuzzedDataProvider& fuzzed_data_provider) noexcep
 
 inline CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept
 {
-    return {ConsumeService(fuzzed_data_provider), static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()), fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
+    return {ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
 }
 
-inline CNode ConsumeNode(FuzzedDataProvider& fuzzed_data_provider) noexcept
+template <bool ReturnUniquePtr = false>
+auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = nullopt) noexcept
 {
-    const NodeId node_id = fuzzed_data_provider.ConsumeIntegral<NodeId>();
-    const ServiceFlags local_services = static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
+    const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegral<NodeId>());
+    const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
     const SOCKET socket = INVALID_SOCKET;
     const CAddress address = ConsumeAddress(fuzzed_data_provider);
     const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
     const uint64_t local_host_nonce = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
     const CAddress addr_bind = ConsumeAddress(fuzzed_data_provider);
     const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64);
-    const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
+    const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES);
     const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
-    return {node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
+    if constexpr (ReturnUniquePtr) {
+        return std::make_unique<CNode>(node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion);
+    } else {
+        return CNode{node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
+    }
 }
+inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = nullopt) { return ConsumeNode<true>(fdp, node_id_in); }
 
 inline void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST)
 {
diff --git a/src/test/util/net.h b/src/test/util/net.h
index 1208e92762..e25036be26 100644
--- a/src/test/util/net.h
+++ b/src/test/util/net.h
@@ -30,4 +30,35 @@ struct ConnmanTestMsg : public CConnman {
     bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const;
 };
 
+constexpr ServiceFlags ALL_SERVICE_FLAGS[]{
+    NODE_NONE,
+    NODE_NETWORK,
+    NODE_BLOOM,
+    NODE_WITNESS,
+    NODE_COMPACT_FILTERS,
+    NODE_NETWORK_LIMITED,
+};
+
+constexpr NetPermissionFlags ALL_NET_PERMISSION_FLAGS[]{
+    NetPermissionFlags::PF_NONE,
+    NetPermissionFlags::PF_BLOOMFILTER,
+    NetPermissionFlags::PF_RELAY,
+    NetPermissionFlags::PF_FORCERELAY,
+    NetPermissionFlags::PF_NOBAN,
+    NetPermissionFlags::PF_MEMPOOL,
+    NetPermissionFlags::PF_ADDR,
+    NetPermissionFlags::PF_DOWNLOAD,
+    NetPermissionFlags::PF_ISIMPLICIT,
+    NetPermissionFlags::PF_ALL,
+};
+
+constexpr ConnectionType ALL_CONNECTION_TYPES[]{
+    ConnectionType::INBOUND,
+    ConnectionType::OUTBOUND_FULL_RELAY,
+    ConnectionType::MANUAL,
+    ConnectionType::FEELER,
+    ConnectionType::BLOCK_RELAY,
+    ConnectionType::ADDR_FETCH,
+};
+
 #endif // BITCOIN_TEST_UTIL_NET_H