Skip to content

Commit 3a6acd1

Browse files
author
MarcoFalke
committed
Merge #20789: fuzz: Rework strong and weak net enum fuzzing
eeee43b fuzz: Use ConsumeWeakEnum for ServiceFlags (MarcoFalke) fa9949b fuzz: Add ConsumeWeakEnum helper, Extract ALL_NET_PERMISSION_FLAGS (MarcoFalke) faaef94 fuzz: [refactor] Extract ALL_CONNECTION_TYPES constant (MarcoFalke) fa42da2 fuzz: Use ConsumeNode in process_message target (MarcoFalke) fa121f0 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 eeee43b after rebase. Tree-SHA512: 1ad9520c7e708b7f4994ae8f77886ffca33d7c542756e2a3e07dbbbe59e360f9fcaccf2e2fb57d9bc731d4aeb4938fb1c5c546e9d2744b007af5626f5cb377fe
2 parents 4b8b71e + eeee43b commit 3a6acd1

File tree

7 files changed

+62
-27
lines changed

7 files changed

+62
-27
lines changed

‎src/test/fuzz/connman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
128128
connman.SetNetworkActive(fuzzed_data_provider.ConsumeBool());
129129
break;
130130
case 26:
131-
connman.SetServices(random_service, static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()));
131+
connman.SetServices(random_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
132132
break;
133133
case 27:
134134
connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool());

‎src/test/fuzz/net.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <test/fuzz/FuzzedDataProvider.h>
1414
#include <test/fuzz/fuzz.h>
1515
#include <test/fuzz/util.h>
16+
#include <test/util/net.h>
1617
#include <test/util/setup_common.h>
1718

1819
#include <cstdint>
@@ -122,9 +123,7 @@ FUZZ_TARGET_INIT(net, initialize_net)
122123
(void)node.GetCommonVersion();
123124
(void)node.RelayAddrsWithConn();
124125

125-
const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ?
126-
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}) :
127-
static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
126+
const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
128127
(void)node.HasPermission(net_permission_flags);
129128
(void)node.ConnectedThroughNetwork();
130129
}

‎src/test/fuzz/net_permissions.cpp

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,7 @@ FUZZ_TARGET(net_permissions)
1717
{
1818
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
1919
const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(32);
20-
const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ? fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({
21-
NetPermissionFlags::PF_NONE,
22-
NetPermissionFlags::PF_BLOOMFILTER,
23-
NetPermissionFlags::PF_RELAY,
24-
NetPermissionFlags::PF_FORCERELAY,
25-
NetPermissionFlags::PF_NOBAN,
26-
NetPermissionFlags::PF_MEMPOOL,
27-
NetPermissionFlags::PF_ADDR,
28-
NetPermissionFlags::PF_ISIMPLICIT,
29-
NetPermissionFlags::PF_ALL,
30-
}) :
31-
static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
20+
const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
3221

3322
NetWhitebindPermissions net_whitebind_permissions;
3423
bilingual_str error_net_whitebind_permissions;

‎src/test/fuzz/process_message.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <streams.h>
1414
#include <test/fuzz/FuzzedDataProvider.h>
1515
#include <test/fuzz/fuzz.h>
16+
#include <test/fuzz/util.h>
1617
#include <test/util/mining.h>
1718
#include <test/util/net.h>
1819
#include <test/util/setup_common.h>
@@ -63,13 +64,15 @@ void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO
6364
}
6465
const bool jump_out_of_ibd{fuzzed_data_provider.ConsumeBool()};
6566
if (jump_out_of_ibd) chainstate.JumpOutOfIbd();
66-
CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
67-
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();
67+
CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release();
6868
p2p_node.fSuccessfullyConnected = true;
6969
p2p_node.nVersion = PROTOCOL_VERSION;
7070
p2p_node.SetCommonVersion(PROTOCOL_VERSION);
7171
connman.AddTestNode(p2p_node);
7272
g_setup->m_node.peerman->InitializeNode(&p2p_node);
73+
74+
// fuzzed_data_provider is fully consumed after this call, don't use it
75+
CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
7376
try {
7477
g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream,
7578
GetTime<std::chrono::microseconds>(), std::atomic<bool>{false});

‎src/test/fuzz/process_messages.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
4747

4848
const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3);
4949
for (int i = 0; i < num_peers_to_add; ++i) {
50-
const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
51-
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
52-
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());
50+
peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release());
5351
CNode& p2p_node = *peers.back();
5452

5553
p2p_node.fSuccessfullyConnected = true;

‎src/test/fuzz/util.h

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <streams.h>
2323
#include <test/fuzz/FuzzedDataProvider.h>
2424
#include <test/fuzz/fuzz.h>
25+
#include <test/util/net.h>
2526
#include <test/util/setup_common.h>
2627
#include <txmempool.h>
2728
#include <uint256.h>
@@ -86,6 +87,14 @@ template <typename T>
8687
return obj;
8788
}
8889

90+
template <typename WeakEnumType, size_t size>
91+
[[nodiscard]] WeakEnumType ConsumeWeakEnum(FuzzedDataProvider& fuzzed_data_provider, const WeakEnumType (&all_types)[size]) noexcept
92+
{
93+
return fuzzed_data_provider.ConsumeBool() ?
94+
fuzzed_data_provider.PickValueInArray<WeakEnumType>(all_types) :
95+
WeakEnumType(fuzzed_data_provider.ConsumeIntegral<typename std::underlying_type<WeakEnumType>::type>());
96+
}
97+
8998
[[nodiscard]] inline opcodetype ConsumeOpcodeType(FuzzedDataProvider& fuzzed_data_provider) noexcept
9099
{
91100
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
283292

284293
inline CAddress ConsumeAddress(FuzzedDataProvider& fuzzed_data_provider) noexcept
285294
{
286-
return {ConsumeService(fuzzed_data_provider), static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()), fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
295+
return {ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS), fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
287296
}
288297

289-
inline CNode ConsumeNode(FuzzedDataProvider& fuzzed_data_provider) noexcept
298+
template <bool ReturnUniquePtr = false>
299+
auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<NodeId>& node_id_in = nullopt) noexcept
290300
{
291-
const NodeId node_id = fuzzed_data_provider.ConsumeIntegral<NodeId>();
292-
const ServiceFlags local_services = static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
301+
const NodeId node_id = node_id_in.value_or(fuzzed_data_provider.ConsumeIntegral<NodeId>());
302+
const ServiceFlags local_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
293303
const SOCKET socket = INVALID_SOCKET;
294304
const CAddress address = ConsumeAddress(fuzzed_data_provider);
295305
const uint64_t keyed_net_group = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
296306
const uint64_t local_host_nonce = fuzzed_data_provider.ConsumeIntegral<uint64_t>();
297307
const CAddress addr_bind = ConsumeAddress(fuzzed_data_provider);
298308
const std::string addr_name = fuzzed_data_provider.ConsumeRandomLengthString(64);
299-
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
309+
const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES);
300310
const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
301-
return {node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
311+
if constexpr (ReturnUniquePtr) {
312+
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);
313+
} else {
314+
return CNode{node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion};
315+
}
302316
}
317+
inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = nullopt) { return ConsumeNode<true>(fdp, node_id_in); }
303318

304319
inline void InitializeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST)
305320
{

‎src/test/util/net.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,35 @@ struct ConnmanTestMsg : public CConnman {
3030
bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg& ser_msg) const;
3131
};
3232

33+
constexpr ServiceFlags ALL_SERVICE_FLAGS[]{
34+
NODE_NONE,
35+
NODE_NETWORK,
36+
NODE_BLOOM,
37+
NODE_WITNESS,
38+
NODE_COMPACT_FILTERS,
39+
NODE_NETWORK_LIMITED,
40+
};
41+
42+
constexpr NetPermissionFlags ALL_NET_PERMISSION_FLAGS[]{
43+
NetPermissionFlags::PF_NONE,
44+
NetPermissionFlags::PF_BLOOMFILTER,
45+
NetPermissionFlags::PF_RELAY,
46+
NetPermissionFlags::PF_FORCERELAY,
47+
NetPermissionFlags::PF_NOBAN,
48+
NetPermissionFlags::PF_MEMPOOL,
49+
NetPermissionFlags::PF_ADDR,
50+
NetPermissionFlags::PF_DOWNLOAD,
51+
NetPermissionFlags::PF_ISIMPLICIT,
52+
NetPermissionFlags::PF_ALL,
53+
};
54+
55+
constexpr ConnectionType ALL_CONNECTION_TYPES[]{
56+
ConnectionType::INBOUND,
57+
ConnectionType::OUTBOUND_FULL_RELAY,
58+
ConnectionType::MANUAL,
59+
ConnectionType::FEELER,
60+
ConnectionType::BLOCK_RELAY,
61+
ConnectionType::ADDR_FETCH,
62+
};
63+
3364
#endif // BITCOIN_TEST_UTIL_NET_H

0 commit comments

Comments
 (0)