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
This commit is contained in:
MarcoFalke 2020-06-03 07:23:10 -04:00 committed by PastaPastaPasta
parent be1ca25e21
commit c585d57fec
2 changed files with 8 additions and 2 deletions

View File

@ -14,6 +14,7 @@
#include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h> #include <test/fuzz/fuzz.h>
#include <test/util/mining.h> #include <test/util/mining.h>
#include <test/util/net.h>
#include <test/util/setup_common.h> #include <test/util/setup_common.h>
#include <validationinterface.h> #include <validationinterface.h>
#include <version.h> #include <version.h>
@ -59,15 +60,17 @@ void initialize_process_message()
void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO_MESSAGE_TYPE) void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO_MESSAGE_TYPE)
{ {
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); 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()}; 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) { if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
return; return;
} }
CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION}; CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
CNode p2p_node{0, ServiceFlags(NODE_NETWORK | NODE_BLOOM), INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, false}; CNode& p2p_node = *std::make_unique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_BLOOM), INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, false).release();
p2p_node.fSuccessfullyConnected = true; p2p_node.fSuccessfullyConnected = true;
p2p_node.nVersion = PROTOCOL_VERSION; p2p_node.nVersion = PROTOCOL_VERSION;
p2p_node.SetSendVersion(PROTOCOL_VERSION); p2p_node.SetSendVersion(PROTOCOL_VERSION);
connman.AddTestNode(p2p_node);
g_setup->m_node.peerman->InitializeNode(&p2p_node); g_setup->m_node.peerman->InitializeNode(&p2p_node);
try { try {
g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), std::atomic<bool>{false}); g_setup->m_node.peerman->ProcessMessage(p2p_node, random_message_type, random_bytes_data_stream, GetTimeMillis(), std::atomic<bool>{false});
@ -78,6 +81,8 @@ void fuzz_target(const std::vector<uint8_t>& buffer, const std::string& LIMIT_TO
g_setup->m_node.peerman->SendMessages(&p2p_node); g_setup->m_node.peerman->SendMessages(&p2p_node);
} }
SyncWithValidationInterfaceQueue(); SyncWithValidationInterfaceQueue();
LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
g_setup->m_node.connman->StopNodes();
} }
FUZZ_TARGET_INIT(process_message, initialize_process_message) { fuzz_target(buffer, ""); } FUZZ_TARGET_INIT(process_message, initialize_process_message) { fuzz_target(buffer, ""); }

View File

@ -77,6 +77,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
g_setup->m_node.peerman->SendMessages(&random_node); g_setup->m_node.peerman->SendMessages(&random_node);
} }
} }
connman.ClearTestNodes();
SyncWithValidationInterfaceQueue(); SyncWithValidationInterfaceQueue();
LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
g_setup->m_node.connman->StopNodes();
} }