diff options
author | moneromooo-monero <moneromooo-monero@users.noreply.github.com> | 2017-12-11 22:36:58 +0000 |
---|---|---|
committer | moneromooo-monero <moneromooo-monero@users.noreply.github.com> | 2017-12-18 15:15:40 +0000 |
commit | 45a1c4c0885584554668c89bd6976474af01e2f5 (patch) | |
tree | 1e43812b37e0f2c2cb967f52133e0ddb09fbb61e | |
parent | tests: fix a buffer overread in a unit test (diff) | |
download | monero-45a1c4c0885584554668c89bd6976474af01e2f5.tar.xz |
add empty container sanity checks when using front() and back()
-rw-r--r-- | contrib/epee/include/net/abstract_tcp_server.h | 2 | ||||
-rw-r--r-- | src/cryptonote_protocol/block_queue.cpp | 1 | ||||
-rw-r--r-- | src/cryptonote_protocol/cryptonote_protocol_handler.inl | 6 | ||||
-rw-r--r-- | src/mnemonics/electrum-words.cpp | 2 | ||||
-rw-r--r-- | src/rpc/core_rpc_server.cpp | 11 | ||||
-rw-r--r-- | src/rpc/daemon_handler.cpp | 4 | ||||
-rw-r--r-- | src/simplewallet/simplewallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet2.cpp | 1 | ||||
-rw-r--r-- | src/wallet/wallet_rpc_server.cpp | 7 |
9 files changed, 32 insertions, 6 deletions
diff --git a/contrib/epee/include/net/abstract_tcp_server.h b/contrib/epee/include/net/abstract_tcp_server.h index 000305cfa..cbad1717c 100644 --- a/contrib/epee/include/net/abstract_tcp_server.h +++ b/contrib/epee/include/net/abstract_tcp_server.h @@ -305,7 +305,7 @@ namespace net_utils m_connections.back().powner = this; m_connections.back().m_self_it = --m_connections.end(); m_connections.back().m_context.m_remote_address = remote_address; - m_connections.back().m_htread = threads_helper::create_thread(ConnectionHandlerProc, &m_connections.back()); + m_connections.back().m_htread = threads_helper::create_thread(ConnectionHandlerProc, &m_connections.back()); // ugh, seems very risky return true; } diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp index bfff35456..9e38ffbcb 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)); } diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index de30df5d7..8aef31a5a 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -1003,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)) @@ -1498,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) { diff --git a/src/mnemonics/electrum-words.cpp b/src/mnemonics/electrum-words.cpp index 1b14905f6..ba67952aa 100644 --- a/src/mnemonics/electrum-words.cpp +++ b/src/mnemonics/electrum-words.cpp @@ -205,6 +205,8 @@ namespace */ bool checksum_test(std::vector<std::string> seed, uint32_t unique_prefix_length) { + if (seed.empty()) + return false; // The last word is the checksum. std::string last_word = seed.back(); seed.pop_back(); diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index a1d4d2d38..a6109cb89 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -492,6 +492,11 @@ namespace cryptonote { if (std::find(missed_txs.begin(), missed_txs.end(), h) == missed_txs.end()) { + if (txs.empty()) + { + res.status = "Failed: internal error - txs is empty"; + return true; + } // core returns the ones it finds in the right order if (get_transaction_hash(txs.front()) != h) { @@ -1150,7 +1155,7 @@ namespace cryptonote error_resp.message = "Internal error: can't get block by hash. Hash = " + req.hash + '.'; return false; } - if (blk.miner_tx.vin.front().type() != typeid(txin_gen)) + if (blk.miner_tx.vin.size() != 1 || blk.miner_tx.vin.front().type() != typeid(txin_gen)) { error_resp.code = CORE_RPC_ERROR_CODE_INTERNAL_ERROR; error_resp.message = "Internal error: coinbase transaction in the block has the wrong type"; @@ -1188,7 +1193,7 @@ namespace cryptonote error_resp.message = "Internal error: can't get block by height. Height = " + boost::lexical_cast<std::string>(h) + ". Hash = " + epee::string_tools::pod_to_hex(block_hash) + '.'; return false; } - if (blk.miner_tx.vin.front().type() != typeid(txin_gen)) + if (blk.miner_tx.vin.size() != 1 || blk.miner_tx.vin.front().type() != typeid(txin_gen)) { error_resp.code = CORE_RPC_ERROR_CODE_INTERNAL_ERROR; error_resp.message = "Internal error: coinbase transaction in the block has the wrong type"; @@ -1274,7 +1279,7 @@ namespace cryptonote error_resp.message = "Internal error: can't get block by hash. Hash = " + req.hash + '.'; return false; } - if (blk.miner_tx.vin.front().type() != typeid(txin_gen)) + if (blk.miner_tx.vin.size() != 1 || blk.miner_tx.vin.front().type() != typeid(txin_gen)) { error_resp.code = CORE_RPC_ERROR_CODE_INTERNAL_ERROR; error_resp.message = "Internal error: coinbase transaction in the block has the wrong type"; diff --git a/src/rpc/daemon_handler.cpp b/src/rpc/daemon_handler.cpp index 6643ce4e4..908f9e187 100644 --- a/src/rpc/daemon_handler.cpp +++ b/src/rpc/daemon_handler.cpp @@ -799,6 +799,10 @@ namespace rpc } header.hash = hash_in; + if (b.miner_tx.vin.size() != 1 || b.miner_tx.vin.front().type() != typeid(txin_gen)) + { + return false; + } header.height = boost::get<txin_gen>(b.miner_tx.vin.front()).height; header.major_version = b.major_version; diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 64e665fb3..e34e718d2 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -4321,9 +4321,9 @@ bool simple_wallet::sweep_single(const std::vector<std::string> &args_) fail_msg_writer() << tr("Multiple transactions are created, which is not supposed to happen"); return true; } - if (ptx_vector[0].selected_transfers.size() > 1) + if (ptx_vector[0].selected_transfers.size() != 1) { - fail_msg_writer() << tr("The transaction uses multiple inputs, which is not supposed to happen"); + fail_msg_writer() << tr("The transaction uses multiple or no inputs, which is not supposed to happen"); return true; } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 04c6ee236..88f9e0934 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -4950,6 +4950,7 @@ bool wallet2::tx_add_fake_output(std::vector<std::vector<tools::wallet2::get_out if (global_index == real_index) // don't re-add real one return false; auto item = std::make_tuple(global_index, tx_public_key, mask); + CHECK_AND_ASSERT_MES(!outs.empty(), false, "internal error: outs is empty"); if (std::find(outs.back().begin(), outs.back().end(), item) != outs.back().end()) // don't add duplicates return false; outs.back().push_back(item); diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 0482b9dd6..baf8086f8 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -602,6 +602,13 @@ namespace tools uint64_t mixin = m_wallet->adjust_mixin(req.mixin); std::vector<wallet2::pending_tx> ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, req.priority, extra, req.account_index, req.subaddr_indices, m_trusted_daemon); + if (ptx_vector.empty()) + { + er.code = WALLET_RPC_ERROR_CODE_TX_NOT_POSSIBLE; + er.message = "No transaction created"; + return false; + } + // reject proposed transactions if there are more than one. see on_transfer_split below. if (ptx_vector.size() != 1) { |