fuzz: apply node context reset pattern to p2p_handshake
What the developer wrote
Authored by frankomosh
fuzz: apply node context reset pattern to p2p_handshake
Apply the node context reset pattern from fabf8d1 to p2p_handshake. Previous pattern created local AddrMan and Warnings objects, leaving connman holding dangling references across iterations. Reset and reinstall node.addrman and node.peerman each iteration so sanitizers can detect stale pointer usage.
What changed, and why it matters
This change fixes a bug in a Bitcoin Core fuzz test (an automated testing harness, not production code). The test was creating new address-manager and peer-manager objects on every fuzzing iteration while leaving the connection manager pointing to the old, destroyed ones. That produced dangling pointers, which could cause crashes or false negatives during fuzzing but does not affect real Bitcoin nodes.
No production action required. Ensure fuzzing infrastructure runs with AddressSanitizer to validate the fix and consider auditing other fuzz targets for the same pattern.
Security signals we found
dangling pointer / use-after-free class bug in test harness
fix follows a previously established reset pattern (fabf8d1)
change is confined to fuzz test code (src/test/fuzz/p2p_handshake.cpp)
no production networking or consensus code modified
Evidence from the diff
The p2p_handshake fuzz target previously declared local AddrMan and node::Warnings objects inside the fuzz loop and built a new PeerManager from them, but did not update the ConnmanTestMsg’s references. Across iterations the old objects were destroyed, leaving connman with dangling references. The patch applies the node context reset pattern from fabf8d1: it resets node.banman, node.addrman, and node.peerman, recreates them on the NodeContext, reinstalls the message processor and addrman in connman, and uses the persistent node.warnings object. This lets sanitizers catch stale pointer usage and prevents UAF-like conditions inside the fuzz harness.
Changed components
src/test/fuzz/p2p_handshake.cppInspect captured patch +18 / −19
diff --git a/src/test/fuzz/p2p_handshake.cpp b/src/test/fuzz/p2p_handshake.cpp
index 421aacd8..85b18476 100644
--- a/src/test/fuzz/p2p_handshake.cpp
+++ b/src/test/fuzz/p2p_handshake.cpp
@@ -2,19 +2,15 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#include <addrman.h>
-#include <consensus/consensus.h>
+#include <banman.h>
#include <net.h>
#include <net_processing.h>
-#include <node/warnings.h>
#include <protocol.h>
-#include <script/script.h>
#include <sync.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <test/fuzz/util/net.h>
-#include <test/util/mining.h>
#include <test/util/net.h>
#include <test/util/setup_common.h>
#include <test/util/time.h>
@@ -23,16 +19,15 @@
#include <validationinterface.h>
#include <ios>
-#include <string>
#include <utility>
#include <vector>
namespace {
-const TestingSetup* g_setup;
+TestingSetup* g_setup;
void initialize()
{
- static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
+ static const auto testing_setup = MakeNoLogFileContext<TestingSetup>(
/*chain_type=*/ChainType::REGTEST);
g_setup = testing_setup.get();
}
@@ -43,22 +38,26 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize)
SeedRandomStateForTest(SeedRand::ZEROS);
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
- auto& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
- auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
+ auto& node{g_setup->m_node};
+ auto& connman{static_cast<ConnmanTestMsg&>(*node.connman)};
+ auto& chainman{static_cast<TestChainstateManager&>(*node.chainman)};
NodeClockContext clock_ctx{1610000000s}; // any time to successfully reset ibd
chainman.ResetIbd();
- node::Warnings warnings{};
- auto netgroupman{NetGroupManager::NoAsmap()};
- AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0};
- auto peerman = PeerManager::make(connman, addrman,
+ node.banman.reset();
+ node.addrman.reset();
+ node.peerman.reset();
+ node.addrman = std::make_unique<AddrMan>(
+ *node.netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0);
+ node.peerman = PeerManager::make(connman, *node.addrman,
/*banman=*/nullptr, chainman,
- *g_setup->m_node.mempool, warnings,
+ *node.mempool, *node.warnings,
PeerManager::Options{
.reconcile_txs = true,
.deterministic_rng = true,
});
- connman.SetMsgProc(peerman.get());
+ connman.SetMsgProc(node.peerman.get());
+ connman.SetAddrman(*node.addrman);
LOCK(NetEventsInterface::g_msgproc_mutex);
@@ -67,7 +66,7 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize)
for (int i = 0; i < num_peers_to_add; ++i) {
peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release());
connman.AddTestNode(*peers.back());
- peerman->InitializeNode(
+ node.peerman->InitializeNode(
*peers.back(),
static_cast<ServiceFlags>(fuzzed_data_provider.ConsumeIntegral<uint64_t>()));
}
@@ -102,9 +101,9 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize)
more_work = connman.ProcessMessagesOnce(connection);
} catch (const std::ios_base::failure&) {
}
- peerman->SendMessages(connection);
+ node.peerman->SendMessages(connection);
}
}
- g_setup->m_node.connman->StopNodes();
+ node.connman->StopNodes();
}
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.