Merge pull request #2920

bd5cce07 network_throttle: fix ineffective locking (moneromooo-monero)
e0a61299 network_throttle: remove unused xxx static member (moneromooo-monero)
24f584d9 cryptonote_core: remove unused functions with off by one bugs (moneromooo-monero)
b1634aa3 blockchain: don't leave dangling pointers in this (moneromooo-monero)
8e60b81c cryptonote_core: fix db leak on error (moneromooo-monero)
213e326c abstract_tcp_server2: log init_server errors as fatal (moneromooo-monero)
b51dc566 use const refs in for loops for non tiny types (moneromooo-monero)
f0568ca6 net_parse_helpers: fix regex error checking (moneromooo-monero)
b49ddc76 check accessing an element past the end of a container (moneromooo-monero)
2305bf26 check return value for generate_key_derivation and derive_public_key (moneromooo-monero)
a4240d9f catch const exceptions (moneromooo-monero)
45a1c4c0 add empty container sanity checks when using front() and back() (moneromooo-monero)
56fa6ce1 tests: fix a buffer overread in a unit test (moneromooo-monero)
b4524892 rpc: guard against json parsing a non object (moneromooo-monero)
c2ed8618 easylogging++: avoid buffer underflow (moneromooo-monero)
187a6ab2 epee: trap failure to parse URI from request (moneromooo-monero)
061789b5 checkpoints: trap failure to load JSON checkpoints (moneromooo-monero)
ba2fefb9 checkpoints: pass std::string by const ref, not const value (moneromooo-monero)
38c8f4e0 mlog: terminate a string at last char, just in case (moneromooo-monero)
d753d716 fix a few leaks by throwing objects, not newed pointers to objects (moneromooo-monero)
fe568db8 p2p: use size_t for arbitrary counters instead of uint8_t (moneromooo-monero)
46d6fa35 cryptonote_protocol: sanity check chain hashes from peer (moneromooo-monero)
25584f86 cryptonote_protocol: print peer versions when unexpected (moneromooo-monero)
490a5d41 rpc: do not try to use an invalid txid in relay_tx (moneromooo-monero)

2 files changed, 24 insertions, 7 deletions

diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp
index bfff35456..3844d3751 100644
--- a/src/cryptonote_protocol/block_queue.cpp
+++ b/src/cryptonote_protocol/block_queue.cpp
@@ -62,6 +62,7 @@ void block_queue::add_blocks(uint64_t height, std::list<cryptonote::block_comple
 
 void block_queue::add_blocks(uint64_t height, uint64_t nblocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time)
 {
+  CHECK_AND_ASSERT_THROW_MES(nblocks > 0, "Empty span");
   boost::unique_lock<boost::recursive_mutex> lock(mutex);
   blocks.insert(span(height, nblocks, connection_id, time));
 }
@@ -384,7 +385,7 @@ float block_queue::get_speed(const boost::uuids::uuid &connection_id) const
       i->second = (i->second + span.rate) / 2;
   }
   float conn_rate = -1, best_rate = 0;
-  for (auto i: speeds)
+  for (const auto &i: speeds)
   {
     if (i.first == connection_id)
       conn_rate = i.second;
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index 389e8ba84..8aef31a5a 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -266,13 +266,17 @@ namespace cryptonote
       return true;
 
     // from v6, if the peer advertises a top block version, reject if it's not what it should be (will only work if no voting)
-    const uint8_t version = m_core.get_ideal_hard_fork_version(hshd.current_height - 1);
-    if (version >= 6 && version != hshd.top_version)
+    if (hshd.current_height > 0)
     {
-      if (version < hshd.top_version)
-        MCLOG_RED(el::Level::Warning, "global", context << " peer claims higher version that we think - we may be forked from the network and a software upgrade may be needed");
-      LOG_DEBUG_CC(context, "Ignoring due to wrong top version for block " << (hshd.current_height - 1) << ": " << (unsigned)hshd.top_version << ", expected " << (unsigned)version);
-      return false;
+      const uint8_t version = m_core.get_ideal_hard_fork_version(hshd.current_height - 1);
+      if (version >= 6 && version != hshd.top_version)
+      {
+        if (version < hshd.top_version)
+          MCLOG_RED(el::Level::Warning, "global", context << " peer claims higher version that we think (" <<
+              (unsigned)hshd.top_version << " for " << (hshd.current_height - 1) << "instead of " << (unsigned)version <<
+              ") - we may be forked from the network and a software upgrade may be needed");
+        return false;
+      }
     }
 
     context.m_remote_blockchain_height = hshd.current_height;
@@ -999,6 +1003,11 @@ skip:
           MDEBUG(context << " next span in the queue has blocks " << start_height << "-" << (start_height + blocks.size() - 1)
               << ", we need " << previous_height);
 
+          if (blocks.empty())
+          {
+            MERROR("Next span has no blocks");
+            break;
+          }
 
           block new_block;
           if (!parse_and_validate_block_from_blob(blocks.front().block, new_block))
@@ -1494,6 +1503,7 @@ skip:
 
       NOTIFY_REQUEST_CHAIN::request r = boost::value_initialized<NOTIFY_REQUEST_CHAIN::request>();
       m_core.get_short_chain_history(r.block_ids);
+      CHECK_AND_ASSERT_MES(!r.block_ids.empty(), false, "Short chain history is empty");
 
       if (!start_from_current_chain)
       {
@@ -1581,6 +1591,12 @@ skip:
       drop_connection(context, true, false);
       return 1;
     }
+    if (arg.total_height < arg.m_block_ids.size() || arg.start_height > arg.total_height - arg.m_block_ids.size())
+    {
+      LOG_ERROR_CCONTEXT("sent invalid start/nblocks/height, dropping connection");
+      drop_connection(context, true, false);
+      return 1;
+    }
 
     context.m_remote_blockchain_height = arg.total_height;
     context.m_last_response_height = arg.start_height + arg.m_block_ids.size()-1;