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)

5 files changed, 18 insertions, 112 deletions

diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index 123bd194b..709c5e852 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -321,6 +321,7 @@ bool Blockchain::init(BlockchainDB* db, const bool testnet, bool offline, const
   if (!db->is_open())
   {
     LOG_ERROR("Attempted to init Blockchain with unopened DB");
+    delete db;
     return false;
   }
 
@@ -471,7 +472,7 @@ bool Blockchain::deinit()
   // memory operation), otherwise we may cause a loop.
   if (m_db == NULL)
   {
-    throw new DB_ERROR("The db pointer is null in Blockchain, the blockchain may be corrupt!");
+    throw DB_ERROR("The db pointer is null in Blockchain, the blockchain may be corrupt!");
   }
 
   try
@@ -489,7 +490,9 @@ bool Blockchain::deinit()
   }
 
   delete m_hardfork;
+  m_hardfork = NULL;
   delete m_db;
+  m_db = NULL;
   return true;
 }
 //------------------------------------------------------------------
@@ -2050,49 +2053,6 @@ bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container
   return true;
 }
 //------------------------------------------------------------------
-void Blockchain::print_blockchain(uint64_t start_index, uint64_t end_index) const
-{
-  LOG_PRINT_L3("Blockchain::" << __func__);
-  std::stringstream ss;
-  CRITICAL_REGION_LOCAL(m_blockchain_lock);
-  auto h = m_db->height();
-  if(start_index > h)
-  {
-    MERROR("Wrong starter index set: " << start_index << ", expected max index " << h);
-    return;
-  }
-
-  for(size_t i = start_index; i <= h && i != end_index; i++)
-  {
-    ss << "height " << i << ", timestamp " << m_db->get_block_timestamp(i) << ", cumul_dif " << m_db->get_block_cumulative_difficulty(i) << ", size " << m_db->get_block_size(i) << "\nid\t\t" << m_db->get_block_hash_from_height(i) << "\ndifficulty\t\t" << m_db->get_block_difficulty(i) << ", nonce " << m_db->get_block_from_height(i).nonce << ", tx_count " << m_db->get_block_from_height(i).tx_hashes.size() << std::endl;
-  }
-  MCINFO("globlal", "Current blockchain:" << std::endl << ss.str());
-}
-//------------------------------------------------------------------
-void Blockchain::print_blockchain_index() const
-{
-  LOG_PRINT_L3("Blockchain::" << __func__);
-  std::stringstream ss;
-  CRITICAL_REGION_LOCAL(m_blockchain_lock);
-  auto height = m_db->height();
-  if (height != 0)
-  {
-    for(uint64_t i = 0; i <= height; i++)
-    {
-      ss << "height: " << i << ", hash: " << m_db->get_block_hash_from_height(i);
-    }
-  }
-
-  MINFO("Current blockchain index:" << std::endl << ss.str());
-}
-//------------------------------------------------------------------
-//TODO: remove this function and references to it
-void Blockchain::print_blockchain_outs(const std::string& file) const
-{
-  LOG_PRINT_L3("Blockchain::" << __func__);
-  return;
-}
-//------------------------------------------------------------------
 // Find the split point between us and foreign blockchain and return
 // (by reference) the most recent common block hash along with up to
 // BLOCKS_IDS_SYNCHRONIZING_DEFAULT_COUNT additional (more recent) hashes.
@@ -2338,7 +2298,7 @@ void Blockchain::on_new_tx_from_block(const cryptonote::transaction &tx)
     TIME_MEASURE_FINISH(a);
     if(m_show_time_stats)
     {
-      size_t ring_size = tx.vin[0].type() == typeid(txin_to_key) ? boost::get<txin_to_key>(tx.vin[0]).key_offsets.size() : 0;
+      size_t ring_size = !tx.vin.empty() && tx.vin[0].type() == typeid(txin_to_key) ? boost::get<txin_to_key>(tx.vin[0]).key_offsets.size() : 0;
       MINFO("HASH: " << "-" << " I/M/O: " << tx.vin.size() << "/" << ring_size << "/" << tx.vout.size() << " H: " << 0 << " chcktx: " << a);
     }
   }
@@ -2373,7 +2333,7 @@ bool Blockchain::check_tx_inputs(transaction& tx, uint64_t& max_used_block_heigh
   TIME_MEASURE_FINISH(a);
   if(m_show_time_stats)
   {
-    size_t ring_size = tx.vin[0].type() == typeid(txin_to_key) ? boost::get<txin_to_key>(tx.vin[0]).key_offsets.size() : 0;
+    size_t ring_size = !tx.vin.empty() && tx.vin[0].type() == typeid(txin_to_key) ? boost::get<txin_to_key>(tx.vin[0]).key_offsets.size() : 0;
     MINFO("HASH: " <<  get_transaction_hash(tx) << " I/M/O: " << tx.vin.size() << "/" << ring_size << "/" << tx.vout.size() << " H: " << max_used_block_height << " ms: " << a + m_fake_scan_time << " B: " << get_object_blobsize(tx));
   }
   if (!res)
@@ -2466,6 +2426,7 @@ bool Blockchain::expand_transaction_2(transaction &tx, const crypto::hash &tx_pr
   // mixRing - full and simple store it in opposite ways
   if (rv.type == rct::RCTTypeFull || rv.type == rct::RCTTypeFullBulletproof)
   {
+    CHECK_AND_ASSERT_MES(!pubkeys.empty() && !pubkeys[0].empty(), false, "empty pubkeys");
     rv.mixRing.resize(pubkeys[0].size());
     for (size_t m = 0; m < pubkeys[0].size(); ++m)
       rv.mixRing[m].clear();
@@ -2480,6 +2441,7 @@ bool Blockchain::expand_transaction_2(transaction &tx, const crypto::hash &tx_pr
   }
   else if (rv.type == rct::RCTTypeSimple || rv.type == rct::RCTTypeSimpleBulletproof)
   {
+    CHECK_AND_ASSERT_MES(!pubkeys.empty() && !pubkeys[0].empty(), false, "empty pubkeys");
     rv.mixRing.resize(pubkeys.size());
     for (size_t n = 0; n < pubkeys.size(); ++n)
     {
@@ -2811,7 +2773,7 @@ bool Blockchain::check_tx_inputs(transaction& tx, tx_verification_context &tvc,
       }
       for (size_t n = 0; n < tx.vin.size(); ++n)
       {
-        if (memcmp(&boost::get<txin_to_key>(tx.vin[n]).k_image, &rv.p.MGs[n].II[0], 32))
+        if (rv.p.MGs[n].II.empty() || memcmp(&boost::get<txin_to_key>(tx.vin[n]).k_image, &rv.p.MGs[n].II[0], 32))
         {
           MERROR_VER("Failed to check ringct signatures: mismatched key image");
           return false;
@@ -2864,7 +2826,7 @@ bool Blockchain::check_tx_inputs(transaction& tx, tx_verification_context &tvc,
         MERROR_VER("Failed to check ringct signatures: Bad MGs size");
         return false;
       }
-      if (rv.p.MGs[0].II.size() != tx.vin.size())
+      if (rv.p.MGs.empty() || rv.p.MGs[0].II.size() != tx.vin.size())
       {
         MERROR_VER("Failed to check ringct signatures: mismatched II/vin sizes");
         return false;
diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h
index b76d0555f..2d5307ac0 100644
--- a/src/cryptonote_core/blockchain.h
+++ b/src/cryptonote_core/blockchain.h
@@ -680,32 +680,6 @@ namespace cryptonote
     //debug functions
 
     /**
-     * @brief prints data about a snippet of the blockchain
-     *
-     * if start_index is greater than the blockchain height, do nothing
-     *
-     * @param start_index height on chain to start at
-     * @param end_index height on chain to end at
-     */
-    void print_blockchain(uint64_t start_index, uint64_t end_index) const;
-
-    /**
-     * @brief prints every block's hash
-     *
-     * WARNING: This function will absolutely crush a terminal in prints, so
-     * it is recommended to redirect this output to a log file (or null sink
-     * if a log file is already set up, as should be the default)
-     */
-    void print_blockchain_index() const;
-
-    /**
-     * @brief currently does nothing, candidate for removal
-     *
-     * @param file
-     */
-    void print_blockchain_outs(const std::string& file) const;
-
-    /**
      * @brief check the blockchain against a set of checkpoints
      *
      * If a block fails a checkpoint and enforce is enabled, the blockchain
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index 415657f9c..da8be18f2 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -377,7 +377,7 @@ namespace cryptonote
     // folder might not be a directory, etc, etc
     catch (...) { }
 
-    BlockchainDB* db = new_db(db_type);
+    std::unique_ptr<BlockchainDB> db(new_db(db_type));
     if (db == NULL)
     {
       LOG_ERROR("Attempted to use non-existent database type");
@@ -468,7 +468,7 @@ namespace cryptonote
     m_blockchain_storage.set_user_options(blocks_threads,
         blocks_per_sync, sync_mode, fast_sync);
 
-    r = m_blockchain_storage.init(db, m_testnet, m_offline, test_options);
+    r = m_blockchain_storage.init(db.release(), m_testnet, m_offline, test_options);
 
     r = m_mempool.init();
     CHECK_AND_ASSERT_MES(r, false, "Failed to initialize memory pool");
@@ -1052,21 +1052,6 @@ namespace cryptonote
     return m_blockchain_storage.find_blockchain_supplement(req_start_block, qblock_ids, blocks, total_height, start_height, max_count);
   }
   //-----------------------------------------------------------------------------------------------
-  void core::print_blockchain(uint64_t start_index, uint64_t end_index) const
-  {
-    m_blockchain_storage.print_blockchain(start_index, end_index);
-  }
-  //-----------------------------------------------------------------------------------------------
-  void core::print_blockchain_index() const
-  {
-    m_blockchain_storage.print_blockchain_index();
-  }
-  //-----------------------------------------------------------------------------------------------
-  void core::print_blockchain_outs(const std::string& file)
-  {
-    m_blockchain_storage.print_blockchain_outs(file);
-  }
-  //-----------------------------------------------------------------------------------------------
   bool core::get_random_outs_for_amounts(const COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::request& req, COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::response& res) const
   {
     return m_blockchain_storage.get_random_outs_for_amounts(req, res);
diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h
index 9f84ed303..adc201fb5 100644
--- a/src/cryptonote_core/cryptonote_core.h
+++ b/src/cryptonote_core/cryptonote_core.h
@@ -601,20 +601,6 @@ namespace cryptonote
      const Blockchain& get_blockchain_storage()const{return m_blockchain_storage;}
 
      /**
-      * @copydoc Blockchain::print_blockchain
-      *
-      * @note see Blockchain::print_blockchain
-      */
-     void print_blockchain(uint64_t start_index, uint64_t end_index) const;
-
-     /**
-      * @copydoc Blockchain::print_blockchain_index
-      *
-      * @note see Blockchain::print_blockchain_index
-      */
-     void print_blockchain_index() const;
-
-     /**
       * @copydoc tx_memory_pool::print_pool
       *
       * @note see tx_memory_pool::print_pool
@@ -622,13 +608,6 @@ namespace cryptonote
      std::string print_pool(bool short_format) const;
 
      /**
-      * @copydoc Blockchain::print_blockchain_outs
-      *
-      * @note see Blockchain::print_blockchain_outs
-      */
-     void print_blockchain_outs(const std::string& file);
-
-     /**
       * @copydoc miner::on_synchronized
       *
       * @note see miner::on_synchronized
diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp
index 89f24a4d4..916b1e05a 100644
--- a/src/cryptonote_core/cryptonote_tx_utils.cpp
+++ b/src/cryptonote_core/cryptonote_tx_utils.cpp
@@ -190,6 +190,12 @@ namespace cryptonote
   //---------------------------------------------------------------
   bool construct_tx_with_tx_key(const account_keys& sender_account_keys, const std::unordered_map<crypto::public_key, subaddress_index>& subaddresses, std::vector<tx_source_entry>& sources, const std::vector<tx_destination_entry>& destinations, const boost::optional<cryptonote::account_public_address>& change_addr, std::vector<uint8_t> extra, transaction& tx, uint64_t unlock_time, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, bool rct, bool bulletproof, rct::multisig_out *msout)
   {
+    if (sources.empty())
+    {
+      LOG_ERROR("Empty sources");
+      return false;
+    }
+
     std::vector<rct::key> amount_keys;
     tx.set_null();
     amount_keys.clear();