Merge pull request #7128

b6b2c9b protocol: stricter checks on received chain hash list (moneromooo)

2 files changed, 36 insertions, 0 deletions

diff --git a/src/cryptonote_basic/connection_context.h b/src/cryptonote_basic/connection_context.h
index 8d26e5638..dfc59631f 100644
--- a/src/cryptonote_basic/connection_context.h
+++ b/src/cryptonote_basic/connection_context.h
@@ -69,6 +69,7 @@ namespace cryptonote
     bool m_anchor;
     int32_t m_score;
     int m_expect_response;
+    uint64_t m_expect_height;
   };
 
   inline std::string get_protocol_state_string(cryptonote_connection_context::state s)
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index 75ca766ba..b537ee254 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -141,6 +141,7 @@ namespace cryptonote
     {
       NOTIFY_REQUEST_CHAIN::request r = {};
       context.m_needed_objects.clear();
+      context.m_expect_height = m_core.get_current_blockchain_height();
       m_core.get_short_chain_history(r.block_ids);
       handler_request_blocks_history( r.block_ids ); // change the limit(?), sleep(?)
       r.prune = m_sync_pruned_blocks;
@@ -493,6 +494,7 @@ namespace cryptonote
       context.m_needed_objects.clear();
       context.m_state = cryptonote_connection_context::state_synchronizing;
       NOTIFY_REQUEST_CHAIN::request r = {};
+      context.m_expect_height = m_core.get_current_blockchain_height();
       m_core.get_short_chain_history(r.block_ids);
       r.prune = m_sync_pruned_blocks;
       handler_request_blocks_history( r.block_ids ); // change the limit(?), sleep(?)
@@ -773,6 +775,7 @@ namespace cryptonote
           context.m_needed_objects.clear();
           context.m_state = cryptonote_connection_context::state_synchronizing;
           NOTIFY_REQUEST_CHAIN::request r = {};
+          context.m_expect_height = m_core.get_current_blockchain_height();
           m_core.get_short_chain_history(r.block_ids);
           handler_request_blocks_history( r.block_ids ); // change the limit(?), sleep(?)
           r.prune = m_sync_pruned_blocks;
@@ -1168,7 +1171,16 @@ namespace cryptonote
         return 1;
       }
       if (start_height == std::numeric_limits<uint64_t>::max())
+      {
         start_height = boost::get<txin_gen>(b.miner_tx.vin[0]).height;
+        if (start_height > context.m_expect_height)
+        {
+          LOG_ERROR_CCONTEXT("sent block ahead of expected height, dropping connection");
+          drop_connection(context, false, false);
+          ++m_sync_bad_spans_downloaded;
+          return 1;
+        }
+      }
 
       auto req_it = context.m_requested_objects.find(block_hash);
       if(req_it == context.m_requested_objects.end())
@@ -1714,6 +1726,7 @@ skip:
             LOG_PRINT_CCONTEXT_L2("requesting callback");
             context.m_last_request_time = boost::date_time::not_a_date_time;
             context.m_expect_response = 0;
+            context.m_expect_height = 0;
             context.m_state = cryptonote_connection_context::state_standby; // we'll go back to adding, then (if we can't), download
             ++context.m_callback_request_count;
             m_p2p->request_callback(context);
@@ -2259,6 +2272,7 @@ skip:
           }
         }
         context.m_last_request_time = boost::posix_time::microsec_clock::universal_time();
+        context.m_expect_height = span.first;
         context.m_expect_response = NOTIFY_RESPONSE_GET_OBJECTS::ID;
         MLOG_P2P_MESSAGE("-->>NOTIFY_REQUEST_GET_OBJECTS: blocks.size()=" << req.blocks.size()
             << "requested blocks count=" << count << " / " << count_limit << " from " << span.first << ", first hash " << req.blocks.front());
@@ -2331,6 +2345,7 @@ skip:
     {//we have to fetch more objects ids, request blockchain entry
 
       NOTIFY_REQUEST_CHAIN::request r = {};
+      context.m_expect_height = m_core.get_current_blockchain_height();
       m_core.get_short_chain_history(r.block_ids);
       CHECK_AND_ASSERT_MES(!r.block_ids.empty(), false, "Short chain history is empty");
 
@@ -2338,7 +2353,10 @@ skip:
       {
         // we'll want to start off from where we are on that peer, which may not be added yet
         if (context.m_last_known_hash != crypto::null_hash && r.block_ids.front() != context.m_last_known_hash)
+        {
+          context.m_expect_height = std::numeric_limits<uint64_t>::max();
           r.block_ids.push_front(context.m_last_known_hash);
+        }
       }
 
       handler_request_blocks_history( r.block_ids ); // change the limit(?), sleep(?)
@@ -2480,6 +2498,12 @@ skip:
       return 1;
     }
     context.m_expect_response = 0;
+    if (arg.start_height + 1 > context.m_expect_height) // we expect an overlapping block
+    {
+      LOG_ERROR_CCONTEXT("Got NOTIFY_RESPONSE_CHAIN_ENTRY past expected height, dropping connection");
+      drop_connection(context, true, false);
+      return 1;
+    }
 
     context.m_last_request_time = boost::date_time::not_a_date_time;
 
@@ -2527,6 +2551,17 @@ skip:
       return 1;
     }
 
+    std::unordered_set<crypto::hash> hashes;
+    for (const auto &h: arg.m_block_ids)
+    {
+      if (!hashes.insert(h).second)
+      {
+        LOG_ERROR_CCONTEXT("sent duplicate block, dropping connection");
+        drop_connection(context, true, false);
+        return 1;
+      }
+    }
+
     uint64_t n_use_blocks = m_core.prevalidate_block_hashes(arg.start_height, arg.m_block_ids, arg.m_block_weights);
     if (n_use_blocks == 0 || n_use_blocks + HASH_OF_HASHES_STEP <= arg.m_block_ids.size())
     {