Remote node DOS

How it was found

Summary:

In monerod, there is an outgoing send queue for each p2p connection to other peers. When this send queue is full, the thread sleeps for approximately 6 seconds and then terminates the connection.

It is possible for an attacker to request enough objects to fill this queue repeatedly, causing all (default = 10) of the p2p threads to sleep.

Additionally, a peer can request objects before performing a handshake, allowing the attacker to bypass any limit set on the number of incoming connections.

This also uses up a lot of memory, causing further issues on memory-constrained systems.

This is related to report #506595

Releases Affected:

All monero versions, including v0.15.0.5

Steps To Reproduce:

• Attacker opens many connections and sends simultaneously sends many NOTIFY_REQUEST_GET_OBJECTS (500 blocks) to <target node>, with specially chosen blocks which can fill the queue For each request:
  • <target node> tries to respond, and fills m_send_que in abstract_tcp_server2.inl
  • The P2P thread in <target node> then sleeps for approximately 6 seconds With enough well-timed requests, this will cause all the P2P threads in <target node> to sleep.

This can be trivially triggered for a single P2P thread by syncing a monerod instance (v0.15.0.5) with the arguments --block-sync-size=500 --limit-rate-down=500 --add-exclusive-node <target node>. Once it reaches a height at which the size of the next 500 blocks is greater than the m_send_que in <target node>, the P2P thread in the target node will sleep for 6 seconds and then drop the connection. This has been tested with a blockchain synced up to height 1675605 (other heights will work).

Quick fix

• Check that a handshake has completed before sending blocks
• Further limit the maximum number of NOTIFY_REQUEST_GET_OBJECTS
• Remove the sleep from the P2P thread when the send queue is full

diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl
index 128ff10aa..e149da480 100644
--- a/contrib/epee/include/net/abstract_tcp_server2.inl
+++ b/contrib/epee/include/net/abstract_tcp_server2.inl
@@ -625,39 +625,12 @@ PRAGMA_WARNING_DISABLE_VS(4355)
     m_send_que_lock.lock(); // *** critical ***
     epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){m_send_que_lock.unlock();});
 
-    long int retry=0;
-    const long int retry_limit = 5*4;
-    while (m_send_que.size() > ABSTRACT_SERVER_SEND_QUE_MAX_COUNT)
-    {
-        retry++;
-
-        /* if ( ::cryptonote::core::get_is_stopping() ) { // TODO re-add fast stop
-            _fact("ABORT queue wait due to stopping");
-            return false; // aborted
-        }*/
-
-        using engine = std::mt19937;
-
-        engine rng;
-        std::random_device dev;
-        std::seed_seq::result_type rand[engine::state_size]{};  // Use complete bit space
-
-        std::generate_n(rand, engine::state_size, std::ref(dev));
-        std::seed_seq seed(rand, rand + engine::state_size);
-        rng.seed(seed);
-
-        long int ms = 250 + (rng() % 50);
-        MDEBUG("Sleeping because QUEUE is FULL, in " << __FUNCTION__ << " for " << ms << " ms before packet_size="<<chunk.size()); // XXX debug sleep
-        m_send_que_lock.unlock();
-        boost::this_thread::sleep(boost::posix_time::milliseconds( ms ) );
-        m_send_que_lock.lock();
-        _dbg1("sleep for queue: " << ms);
 
-        if (retry > retry_limit) {
-            MWARNING("send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection");
-            shutdown();
-            return false;
-        }
+    if (m_send_que.size() > ABSTRACT_SERVER_SEND_QUE_MAX_COUNT)
+    {
+      MWARNING("send que size is more than ABSTRACT_SERVER_SEND_QUE_MAX_COUNT(" << ABSTRACT_SERVER_SEND_QUE_MAX_COUNT << "), shutting down connection");
+      shutdown();
+      return false;
     }
 
     m_send_que.push_back(std::move(chunk));
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.h b/src/cryptonote_protocol/cryptonote_protocol_handler.h
index 3b456e324..6160c6cca 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.h
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.h
@@ -52,7 +52,7 @@ PUSH_WARNINGS
 DISABLE_VS_WARNINGS(4355)
 
 #define LOCALHOST_INT 2130706433
-#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 500
+#define CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT 20
 
 namespace cryptonote
 {
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index 74ceeb41d..e4e15efb3 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -935,6 +935,12 @@ namespace cryptonote
   int t_cryptonote_protocol_handler<t_core>::handle_request_get_objects(int command, NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context)
   {
     MLOG_P2P_MESSAGE("Received NOTIFY_REQUEST_GET_OBJECTS (" << arg.blocks.size() << " blocks)");
+    if(context.m_state == cryptonote_connection_context::state_before_handshake) {
+      LOG_ERROR_CCONTEXT("Requested objects before handshake, dropping connection");
+      drop_connection(context, false, false);
+      return 1;
+    }
+
     if (arg.blocks.size() > CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT)
       {
         LOG_ERROR_CCONTEXT(

Impact

Slowdown of node and possible memory exhaustion on memory constrained systems