Merge pull request monero-project#3226

e464637 keccak: fix mdlen bounds sanity checking (moneromooo-monero)
2e3e90a pass large parameters by const ref, not value (moneromooo-monero)
61defd8 blockchain: sanity check number of precomputed hash of hash blocks (moneromooo-monero)
9af6b2d ringct: fix infinite loop in unused h2b function (moneromooo-monero)
8cea8d0 simplewallet: double check a new multisig wallet is multisig (moneromooo-monero)
9b98a6a threadpool: catch exceptions in dtor, to avoid terminate (moneromooo-monero)
24803ed blockchain_export: fix buffer overflow in exporter (moneromooo-monero)
f3f7da6 perf_timer: rewrite to make it clear there is no division by zero (moneromooo-monero)
c6ea3df performance_tests: remove add_arg call stray extra param (moneromooo-monero)
fa6b456 fuzz_tests: fix an uninitialized var in setup (moneromooo-monero)
03887f1 keccak: fix sanity check bounds test (moneromooo-monero)
ad11db9 blockchain_db: initialize m_open in base class ctor (moneromooo-monero)
bece67f miner: restore std::cout precision after modification (moneromooo-monero)
1aabd14 db_lmdb: check hard fork info drop succeeded (moneromooo-monero)

contrib/epee/include/net/abstract_tcp_server2.h

@@ -207,7 +207,7 @@ namespace net_utils
 
     bool connect(const std::string& adr, const std::string& port, uint32_t conn_timeot, t_connection_context& cn, const std::string& bind_ip = "0.0.0.0");
     template<class t_callback>
-    bool connect_async(const std::string& adr, const std::string& port, uint32_t conn_timeot, t_callback cb, const std::string& bind_ip = "0.0.0.0");
+    bool connect_async(const std::string& adr, const std::string& port, uint32_t conn_timeot, const t_callback &cb, const std::string& bind_ip = "0.0.0.0");
 
     typename t_protocol_handler::config_type& get_config_object(){return m_config;}
 

contrib/epee/include/net/abstract_tcp_server2.inl

@@ -1058,7 +1058,7 @@ POP_WARNINGS
   }
   //---------------------------------------------------------------------------------
   template<class t_protocol_handler> template<class t_callback>
-  bool boosted_tcp_server<t_protocol_handler>::connect_async(const std::string& adr, const std::string& port, uint32_t conn_timeout, t_callback cb, const std::string& bind_ip)
+  bool boosted_tcp_server<t_protocol_handler>::connect_async(const std::string& adr, const std::string& port, uint32_t conn_timeout, const t_callback &cb, const std::string& bind_ip)
   {
     TRY_ENTRY();    
     connection_ptr new_connection_l(new connection<t_protocol_handler>(io_service_, m_config, m_sock_count, m_sock_number, m_pfilter, m_connection_type) );

contrib/epee/include/net/levin_protocol_handler_async.h

@@ -86,16 +86,16 @@ class async_protocol_handler_config
 
   int invoke(int command, const std::string& in_buff, std::string& buff_out, boost::uuids::uuid connection_id);
   template<class callback_t>
-  int invoke_async(int command, const std::string& in_buff, boost::uuids::uuid connection_id, callback_t cb, size_t timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED);
+  int invoke_async(int command, const std::string& in_buff, boost::uuids::uuid connection_id, const callback_t &cb, size_t timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED);
 
   int notify(int command, const std::string& in_buff, boost::uuids::uuid connection_id);
   bool close(boost::uuids::uuid connection_id);
   bool update_connection_context(const t_connection_context& contxt);
   bool request_callback(boost::uuids::uuid connection_id);
   template<class callback_t>
-  bool foreach_connection(callback_t cb);
+  bool foreach_connection(const callback_t &cb);
   template<class callback_t>
-  bool for_connection(const boost::uuids::uuid &connection_id, callback_t cb);
+  bool for_connection(const boost::uuids::uuid &connection_id, const callback_t &cb);
   size_t get_connections_count();
   void set_handler(levin_commands_handler<t_connection_context>* handler, void (*destroy)(levin_commands_handler<t_connection_context>*) = NULL);
 
@@ -248,7 +248,7 @@ class async_protocol_handler
   std::list<boost::shared_ptr<invoke_response_handler_base> > m_invoke_response_handlers;
 
   template<class callback_t>
-  bool add_invoke_response_handler(callback_t cb, uint64_t timeout,  async_protocol_handler& con, int command)
+  bool add_invoke_response_handler(const callback_t &cb, uint64_t timeout,  async_protocol_handler& con, int command)
   {
     CRITICAL_REGION_LOCAL(m_invoke_response_handlers_lock);
     boost::shared_ptr<invoke_response_handler_base> handler(boost::make_shared<anvoke_handler<callback_t>>(cb, timeout, con, command));
@@ -532,7 +532,7 @@ class async_protocol_handler
   }
 
   template<class callback_t>
-  bool async_invoke(int command, const std::string& in_buff, callback_t cb, size_t timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED)
+  bool async_invoke(int command, const std::string& in_buff, const callback_t &cb, size_t timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED)
   {
     misc_utils::auto_scope_leave_caller scope_exit_handler = misc_utils::create_scope_leave_handler(
       boost::bind(&async_protocol_handler::finish_outer_call, this));
@@ -817,15 +817,15 @@ int async_protocol_handler_config<t_connection_context>::invoke(int command, con
 }
 //------------------------------------------------------------------------------------------
 template<class t_connection_context> template<class callback_t>
-int async_protocol_handler_config<t_connection_context>::invoke_async(int command, const std::string& in_buff, boost::uuids::uuid connection_id, callback_t cb, size_t timeout)
+int async_protocol_handler_config<t_connection_context>::invoke_async(int command, const std::string& in_buff, boost::uuids::uuid connection_id, const callback_t &cb, size_t timeout)
 {
   async_protocol_handler<t_connection_context>* aph;
   int r = find_and_lock_connection(connection_id, aph);
   return LEVIN_OK == r ? aph->async_invoke(command, in_buff, cb, timeout) : r;
 }
 //------------------------------------------------------------------------------------------
 template<class t_connection_context> template<class callback_t>
-bool async_protocol_handler_config<t_connection_context>::foreach_connection(callback_t cb)
+bool async_protocol_handler_config<t_connection_context>::foreach_connection(const callback_t &cb)
 {
   CRITICAL_REGION_LOCAL(m_connects_lock);
   for(auto& c: m_connects)
@@ -838,7 +838,7 @@ bool async_protocol_handler_config<t_connection_context>::foreach_connection(cal
 }
 //------------------------------------------------------------------------------------------
 template<class t_connection_context> template<class callback_t>
-bool async_protocol_handler_config<t_connection_context>::for_connection(const boost::uuids::uuid &connection_id, callback_t cb)
+bool async_protocol_handler_config<t_connection_context>::for_connection(const boost::uuids::uuid &connection_id, const callback_t &cb)
 {
   CRITICAL_REGION_LOCAL(m_connects_lock);
   async_protocol_handler<t_connection_context>* aph = find_connection(connection_id);

contrib/epee/include/storages/levin_abstract_invoke2.h

@@ -108,7 +108,7 @@ namespace epee
     }
 
     template<class t_result, class t_arg, class callback_t, class t_transport>
-    bool async_invoke_remote_command2(boost::uuids::uuid conn_id, int command, const t_arg& out_struct, t_transport& transport, callback_t cb, size_t inv_timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED)
+    bool async_invoke_remote_command2(boost::uuids::uuid conn_id, int command, const t_arg& out_struct, t_transport& transport, const callback_t &cb, size_t inv_timeout = LEVIN_DEFAULT_TIMEOUT_PRECONFIGURED)
     {
       typename serialization::portable_storage stg;
       const_cast<t_arg&>(out_struct).store(stg);//TODO: add true const support to searilzation

src/blockchain_db/berkeleydb/db_bdb.cpp

@@ -770,13 +770,13 @@ BlockchainBDB::~BlockchainBDB()
 }
 
 BlockchainBDB::BlockchainBDB(bool batch_transactions) :
+        BlockchainDB(),
         m_buffer(DB_BUFFER_COUNT, DB_BUFFER_LENGTH)
 {
     LOG_PRINT_L3("BlockchainBDB::" << __func__);
     // initialize folder to something "safe" just in case
     // someone accidentally misuses this class...
     m_folder = "thishsouldnotexistbecauseitisgibberish";
-    m_open = false;
     m_run_checkpoint = 0;
     m_batch_transactions = batch_transactions;
     m_write_txn = nullptr;

src/blockchain_db/blockchain_db.h

@@ -537,6 +537,11 @@ class BlockchainDB
 
 public:
 
+  /**
+   * @brief An empty constructor.
+   */
+  BlockchainDB(): m_open(false) { }
+
   /**
    * @brief An empty destructor.
    */

src/blockchain_db/lmdb/db_lmdb.cpp

@@ -1080,13 +1080,12 @@ BlockchainLMDB::~BlockchainLMDB()
     close();
 }
 
-BlockchainLMDB::BlockchainLMDB(bool batch_transactions)
+BlockchainLMDB::BlockchainLMDB(bool batch_transactions): BlockchainDB()
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   // initialize folder to something "safe" just in case
   // someone accidentally misuses this class...
   m_folder = "thishsouldnotexistbecauseitisgibberish";
-  m_open = false;
 
   m_batch_transactions = batch_transactions;
   m_write_txn = nullptr;
@@ -3147,8 +3146,12 @@ void BlockchainLMDB::drop_hard_fork_info()
 
   TXN_PREFIX(0);
 
-  mdb_drop(*txn_ptr, m_hf_starting_heights, 1);
-  mdb_drop(*txn_ptr, m_hf_versions, 1);
+  auto result = mdb_drop(*txn_ptr, m_hf_starting_heights, 1);
+  if (result)
+    throw1(DB_ERROR(lmdb_error("Error dropping hard fork starting heights db: ", result).c_str()));
+  result = mdb_drop(*txn_ptr, m_hf_versions, 1);
+  if (result)
+    throw1(DB_ERROR(lmdb_error("Error dropping hard fork versions db: ", result).c_str()));
 
   TXN_POSTFIX_SUCCESS();
 }

src/blockchain_utilities/blocksdat_file.cpp

@@ -106,7 +106,7 @@ void BlocksdatFile::write_block(const crypto::hash& block_hash)
   {
     crypto::hash hash;
     crypto::cn_fast_hash(m_hashes.data(), HASH_OF_HASHES_STEP * sizeof(crypto::hash), hash);
-    memmove(m_hashes.data(), m_hashes.data() + HASH_OF_HASHES_STEP * sizeof(crypto::hash), (m_hashes.size() - HASH_OF_HASHES_STEP) * sizeof(crypto::hash));
+    memmove(m_hashes.data(), m_hashes.data() + HASH_OF_HASHES_STEP, (m_hashes.size() - HASH_OF_HASHES_STEP) * sizeof(crypto::hash));
     m_hashes.resize(m_hashes.size() - HASH_OF_HASHES_STEP);
     const std::string data(hash.data, sizeof(hash));
     *m_raw_data_file << data;

src/common/perf_timer.cpp

@@ -49,16 +49,15 @@ namespace
 #ifdef __x86_64__
   uint64_t get_ticks_per_ns()
   {
-    uint64_t t0 = epee::misc_utils::get_ns_count();
+    uint64_t t0 = epee::misc_utils::get_ns_count(), t1;
     uint64_t r0 = get_tick_count();
 
     while (1)
     {
-      uint64_t t = epee::misc_utils::get_ns_count();
-      if (t - t0 > 1*1000000000) break; // work one second
+      t1 = epee::misc_utils::get_ns_count();
+      if (t1 - t0 > 1*1000000000) break; // work one second
     }
 
-    uint64_t t1 = epee::misc_utils::get_ns_count();
     uint64_t r1 = get_tick_count();
     uint64_t tpns256 = 256 * (r1 - r0) / (t1 - t0);
     return tpns256 ? tpns256 : 1;

src/common/threadpool.cpp

@@ -25,6 +25,7 @@
 // INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
 // STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
 // THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#include "misc_log_ex.h"
 #include "common/threadpool.h"
 
 #include <cassert>
@@ -81,6 +82,23 @@ int threadpool::get_max_concurrency() {
   return max;
 }
 
+threadpool::waiter::~waiter()
+{
+  {
+    boost::unique_lock<boost::mutex> lock(mt);
+    if (num)
+      MERROR("wait should have been called before waiter dtor - waiting now");
+  }
+  try
+  {
+    wait();
+  }
+  catch (const std::exception &e)
+  {
+    /* ignored */
+  }
+}
+
 void threadpool::waiter::wait() {
   boost::unique_lock<boost::mutex> lock(mt);
   while(num) cv.wait(lock);

src/common/threadpool.h

@@ -34,6 +34,7 @@
 #include <functional>
 #include <utility>
 #include <vector>
+#include <stdexcept>
 
 namespace tools
 {
@@ -57,7 +58,7 @@ class threadpool
     void dec();
     void wait();  //! Wait for a set of tasks to finish.
     waiter() : num(0){}
-    ~waiter() { wait(); }
+    ~waiter();
   };
 
   // Submit a task to the pool. The waiter pointer may be

src/crypto/keccak.c

@@ -92,7 +92,8 @@ void keccak(const uint8_t *in, size_t inlen, uint8_t *md, int mdlen)
     uint8_t temp[144];
     size_t i, rsiz, rsizw;
 
-    if (mdlen <= 0 || mdlen > 200 || sizeof(st) != 200)
+    static_assert(HASH_DATA_AREA <= sizeof(temp), "Bad keccak preconditions");
+    if (mdlen <= 0 || (mdlen > 100 && sizeof(st) != (size_t)mdlen))
     {
       local_abort("Bad keccak use");
     }
@@ -109,7 +110,7 @@ void keccak(const uint8_t *in, size_t inlen, uint8_t *md, int mdlen)
     }
 
     // last block and padding
-    if (inlen >= sizeof(temp) || inlen > rsiz || rsiz - inlen + inlen + 1 >= sizeof(temp) || rsiz == 0 || rsiz - 1 >= sizeof(temp) || rsizw * 8 > sizeof(temp))
+    if (inlen + 1 >= sizeof(temp) || inlen > rsiz || rsiz - inlen + inlen + 1 >= sizeof(temp) || rsiz == 0 || rsiz - 1 >= sizeof(temp) || rsizw * 8 > sizeof(temp))
     {
       local_abort("Bad keccak use");
     }

src/cryptonote_basic/miner.cpp

@@ -198,7 +198,8 @@ namespace cryptonote
       {
         uint64_t total_hr = std::accumulate(m_last_hash_rates.begin(), m_last_hash_rates.end(), 0);
         float hr = static_cast<float>(total_hr)/static_cast<float>(m_last_hash_rates.size());
-        std::cout << "hashrate: " << std::setprecision(4) << std::fixed << hr << ENDL;
+        const auto precision = std::cout.precision();
+        std::cout << "hashrate: " << std::setprecision(4) << std::fixed << hr << precision << ENDL;
       }
     }
     m_last_hr_merge_time = misc_utils::get_tick_count();

src/cryptonote_core/blockchain.cpp

@@ -4348,8 +4348,13 @@ void Blockchain::load_compiled_in_block_hashes()
     {
       const unsigned char *p = get_blocks_dat_start(m_testnet);
       const uint32_t nblocks = *p | ((*(p+1))<<8) | ((*(p+2))<<16) | ((*(p+3))<<24);
+      if (nblocks > (std::numeric_limits<uint32_t>::max() - 4) / sizeof(hash))
+      {
+        MERROR("Block hash data is too large");
+        return;
+      }
       const size_t size_needed = 4 + nblocks * sizeof(crypto::hash);
-      if(nblocks > 0 && nblocks * HASH_OF_HASHES_STEP > m_db->height() && get_blocks_dat_size(m_testnet) >= size_needed)
+      if(nblocks > 0 && nblocks > (m_db->height() + HASH_OF_HASHES_STEP - 1) / HASH_OF_HASHES_STEP && get_blocks_dat_size(m_testnet) >= size_needed)
       {
         p += sizeof(uint32_t);
         m_blocks_hash_of_hashes.reserve(nblocks);

src/p2p/net_node.h

@@ -217,7 +217,7 @@ namespace nodetool
     void add_upnp_port_mapping(uint32_t port);
     void delete_upnp_port_mapping(uint32_t port);
     template<class t_callback>
-    bool try_ping(basic_node_data& node_data, p2p_connection_context& context, t_callback cb);
+    bool try_ping(basic_node_data& node_data, p2p_connection_context& context, const t_callback &cb);
     bool try_get_support_flags(const p2p_connection_context& context, std::function<void(p2p_connection_context&, const uint32_t&)> f);
     bool make_expected_connections_count(PeerType peer_type, size_t expected_connections);
     void cache_connect_fail_info(const epee::net_utils::network_address& addr);

src/p2p/net_node.inl

@@ -1505,7 +1505,7 @@ namespace nodetool
   }
   //-----------------------------------------------------------------------------------
   template<class t_payload_net_handler> template<class t_callback>
-  bool node_server<t_payload_net_handler>::try_ping(basic_node_data& node_data, p2p_connection_context& context, t_callback cb)
+  bool node_server<t_payload_net_handler>::try_ping(basic_node_data& node_data, p2p_connection_context& context, const t_callback &cb)
   {
     if(!node_data.my_port)
       return false;

src/ringct/rctTypes.cpp

@@ -178,6 +178,7 @@ namespace rct {
             }
             while (i < 8 * (j + 1)) {
                 amountb2[i] = 0;
+                i++;
             }
         }
     }

src/simplewallet/simplewallet.cpp

@@ -817,7 +817,11 @@ bool simple_wallet::make_multisig(const std::vector<std::string> &args)
   }
 
   uint32_t total;
-  m_wallet->multisig(NULL, &threshold, &total);
+  if (!m_wallet->multisig(NULL, &threshold, &total))
+  {
+    fail_msg_writer() << tr("Error creating multisig: new wallet is not multisig");
+    return true;
+  }
   success_msg_writer() << std::to_string(threshold) << "/" << total << tr(" multisig address: ")
       << m_wallet->get_account().get_public_address_str(m_wallet->testnet());
 

tests/core_tests/block_validation.cpp

@@ -38,7 +38,7 @@ namespace
 {
   bool lift_up_difficulty(std::vector<test_event_entry>& events, std::vector<uint64_t>& timestamps,
                           std::vector<difficulty_type>& cummulative_difficulties, test_generator& generator,
-                          size_t new_block_count, const block blk_last, const account_base& miner_account)
+                          size_t new_block_count, const block &blk_last, const account_base& miner_account)
   {
     difficulty_type commulative_diffic = cummulative_difficulties.empty() ? 0 : cummulative_difficulties.back();
     block blk_prev = blk_last;

tests/fuzz/levin.cpp

@@ -299,18 +299,19 @@ int LevinFuzzer::run(const std::string &filename)
 {
   std::string s;
 
-//
+#if 0
   epee::levin::bucket_head2 req_head;
   req_head.m_signature = LEVIN_SIGNATURE;
   req_head.m_cb = 0;
   req_head.m_have_to_return_data = true;
   req_head.m_command = 2000;
   req_head.m_flags = LEVIN_PACKET_REQUEST;
   req_head.m_protocol_version = LEVIN_PROTOCOL_VER_1;
+  req_head.m_return_code = 0;
   FILE *f=fopen("/tmp/out.levin", "w");
   fwrite(&req_head,sizeof(req_head),1, f);
   fclose(f);
-//
+#endif
   if (!epee::file_io_utils::load_file_to_string(filename, s))
   {
     std::cout << "Error: failed to load file " << filename << std::endl;

tests/performance_tests/main.cpp

@@ -85,7 +85,7 @@ int main(int argc, char** argv)
 
   po::options_description desc_options("Command line options");
   const command_line::arg_descriptor<std::string> arg_filter = { "filter", "Regular expression filter for which tests to run" };
-  command_line::add_arg(desc_options, arg_filter, "");
+  command_line::add_arg(desc_options, arg_filter);
 
   po::variables_map vm;
   bool r = command_line::handle_error_helper(desc_options, [&]()