aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMarcoFalke <falke.marco@gmail.com>2020-06-03 07:23:10 -0400
committerMarcoFalke <falke.marco@gmail.com>2020-06-03 07:23:41 -0400
commit0f55294cc165e52a87657d7b77614b27c996d638 (patch)
tree9a2e1e86c83a960fb8c8b954dea79f8a09574709
parent42065518d937e036cce6b0ad21890f8fc2c50cfc (diff)
parentfab860aed4878b831dae463e1ee68029b66210f5 (diff)
Merge #18875: fuzz: Stop nodes in process_message* fuzzers
fab860aed4878b831dae463e1ee68029b66210f5 fuzz: Stop nodes in process_message* fuzzers (MarcoFalke) 6666c828e072a5e99ea0c16394ca3e5b9de07409 fuzz: Give CNode ownership to ConnmanTestMsg in process_message fuzz harness (MarcoFalke) Pull request description: Background is that I saw an integer overflow in net_processing ``` #30629113 REDUCE cov: 25793 ft: 142917 corp: 3421/2417Kb lim: 4096 exec/s: 89 rss: 614Mb L: 1719/4096 MS: 1 EraseBytes- net_processing.cpp:977:25: runtime error: signed integer overflow: 2147483624 + 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:977:25 in net_processing.cpp:985:9: runtime error: signed integer overflow: -2147483572 - 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:985:9 in ``` Telling from the line numbers, it looks like `nMisbehavior` wrapped around. Fix that by calling `StopNodes` after each exec, which should clear the node state and thus `nMisbehavior`. ACKs for top commit: practicalswift: ACK fab860aed4878b831dae463e1ee68029b66210f5 Tree-SHA512: 891c081d5843565d891aec028b6c27ef3fa39bc40ae78238e81d8f784b4d4b49cb870998574725a5159dd03aeeb2e0b9bc3d3bb51d57d1231ef42e3394b2d639
-rw-r--r--src/test/fuzz/process_message.cpp7
-rw-r--r--src/test/fuzz/process_messages.cpp3
2 files changed, 8 insertions, 2 deletions
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index 665a6224b4..c51ee3cf29 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -14,6 +14,7 @@
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/util/mining.h>
+#include <test/util/net.h>
#include <test/util/setup_common.h>
#include <util/memory.h>
#include <validationinterface.h>
@@ -63,19 +64,23 @@ void initialize()
void test_one_input(const std::vector<uint8_t>& buffer)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+ ConnmanTestMsg& connman = *(ConnmanTestMsg*)g_setup->m_node.connman.get();
const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()};
if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
return;
}
CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
- CNode p2p_node{0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, false};
+ CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, false).release();
p2p_node.fSuccessfullyConnected = true;
p2p_node.nVersion = PROTOCOL_VERSION;
p2p_node.SetSendVersion(PROTOCOL_VERSION);
+ connman.AddTestNode(p2p_node);
g_setup->m_node.peer_logic->InitializeNode(&p2p_node);
try {
(void)ProcessMessage(&p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), Params(), *g_setup->m_node.chainman, *g_setup->m_node.mempool, g_setup->m_node.connman.get(), g_setup->m_node.banman.get(), std::atomic<bool>{false});
} catch (const std::ios_base::failure&) {
}
SyncWithValidationInterfaceQueue();
+ LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
+ g_setup->m_node.connman->StopNodes();
}
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index bcbf65bdca..ad6c115a49 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -75,6 +75,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
} catch (const std::ios_base::failure&) {
}
}
- connman.ClearTestNodes();
SyncWithValidationInterfaceQueue();
+ LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
+ g_setup->m_node.connman->StopNodes();
}