From fa99e33aebed0109630474e11183b0726b410c2e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 23 Jan 2021 19:32:20 +0100 Subject: [PATCH 1/2] fuzz: move-only FillNode implementation to cpp file This allows to modify the implementation without having to recompile all fuzz targets. Can be reviewed with --color-moved=dimmed-zebra --- src/Makefile.test_fuzz.include | 1 + src/test/fuzz/util.cpp | 22 ++++++++++++++++++++++ src/test/fuzz/util.h | 17 +---------------- 3 files changed, 24 insertions(+), 16 deletions(-) create mode 100644 src/test/fuzz/util.cpp diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 4e858979feb..75fe68fcd19 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -16,6 +16,7 @@ libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAG libtest_fuzz_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libtest_fuzz_a_SOURCES = \ test/fuzz/fuzz.cpp \ + test/fuzz/util.cpp \ $(TEST_FUZZ_H) LIBTEST_FUZZ += $(LIBBITCOIN_SERVER) diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp new file mode 100644 index 00000000000..dbd4f5798c3 --- /dev/null +++ b/src/test/fuzz/util.cpp @@ -0,0 +1,22 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional& version_in) noexcept +{ + const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); + const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); + const int32_t version = version_in.value_or(fuzzed_data_provider.ConsumeIntegral()); + const bool filter_txs = fuzzed_data_provider.ConsumeBool(); + + node.nServices = remote_services; + node.m_permissionFlags = permission_flags; + node.nVersion = version; + node.SetCommonVersion(version); + if (node.m_tx_relay != nullptr) { + LOCK(node.m_tx_relay->cs_filter); + node.m_tx_relay->fRelayTxes = filter_txs; + } +} diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 7796f77cc6c..a6451cf50e9 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -322,22 +322,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional& node_id_in = nullopt) { return ConsumeNode(fdp, node_id_in); } -inline void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional& version_in = std::nullopt) noexcept -{ - const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); - const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); - const int32_t version = version_in.value_or(fuzzed_data_provider.ConsumeIntegral()); - const bool filter_txs = fuzzed_data_provider.ConsumeBool(); - - node.nServices = remote_services; - node.m_permissionFlags = permission_flags; - node.nVersion = version; - node.SetCommonVersion(version); - if (node.m_tx_relay != nullptr) { - LOCK(node.m_tx_relay->cs_filter); - node.m_tx_relay->fRelayTxes = filter_txs; - } -} +void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional& version_in = std::nullopt) noexcept; template std::unique_ptr MakeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector& extra_args = {}) From fad3d7625aa1c2b6c343946e709e87e7168f9d9d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 23 Jan 2021 19:39:30 +0100 Subject: [PATCH 2/2] fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION --- src/test/fuzz/process_message.cpp | 6 ++++-- src/test/fuzz/process_messages.cpp | 5 +++-- src/test/fuzz/util.cpp | 11 +++++++---- src/test/fuzz/util.h | 4 ++-- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 5d6a33d7c2a..e7cc0f52973 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -60,10 +60,12 @@ void fuzz_target(const std::vector& buffer, const std::string& LIMIT_TO return; } CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release(); - FillNode(fuzzed_data_provider, p2p_node); - p2p_node.fSuccessfullyConnected = true; + + const bool successfully_connected{true}; + p2p_node.fSuccessfullyConnected = successfully_connected; connman.AddTestNode(p2p_node); g_setup->m_node.peerman->InitializeNode(&p2p_node); + FillNode(fuzzed_data_provider, p2p_node, /* init_version */ successfully_connected); const auto mock_time = ConsumeTime(fuzzed_data_provider); SetMockTime(mock_time); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index d0d0e19694c..810f0aac92a 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -45,11 +45,12 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) for (int i = 0; i < num_peers_to_add; ++i) { peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release()); CNode& p2p_node = *peers.back(); - FillNode(fuzzed_data_provider, p2p_node); - p2p_node.fSuccessfullyConnected = true; + const bool successfully_connected{true}; + p2p_node.fSuccessfullyConnected = successfully_connected; p2p_node.fPauseSend = false; g_setup->m_node.peerman->InitializeNode(&p2p_node); + FillNode(fuzzed_data_provider, p2p_node, /* init_version */ successfully_connected); connman.AddTestNode(p2p_node); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index dbd4f5798c3..0a541e4186c 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,18 +3,21 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional& version_in) noexcept +void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept { const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); - const int32_t version = version_in.value_or(fuzzed_data_provider.ConsumeIntegral()); + const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange(MIN_PEER_PROTO_VERSION, std::numeric_limits::max()); const bool filter_txs = fuzzed_data_provider.ConsumeBool(); node.nServices = remote_services; node.m_permissionFlags = permission_flags; - node.nVersion = version; - node.SetCommonVersion(version); + if (init_version) { + node.nVersion = version; + node.SetCommonVersion(std::min(version, PROTOCOL_VERSION)); + } if (node.m_tx_relay != nullptr) { LOCK(node.m_tx_relay->cs_filter); node.m_tx_relay->fRelayTxes = filter_txs; diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index a6451cf50e9..4ca3cc68e38 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -320,9 +320,9 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional& node_id_in = nullopt) { return ConsumeNode(fdp, node_id_in); } +inline std::unique_ptr ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional& node_id_in = std::nullopt) { return ConsumeNode(fdp, node_id_in); } -void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional& version_in = std::nullopt) noexcept; +void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept; template std::unique_ptr MakeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector& extra_args = {})