Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DoS/RPC fixes #4438

Merged
merged 3 commits into from
Sep 25, 2018
Merged

DoS/RPC fixes #4438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45683ee
epee: fix invalid memory write reading an array entry
moneromooo-monero Aug 2, 2018
bf9a0f4
epee: fix stack overflow on crafted input
moneromooo-monero Aug 5, 2018
e350cc5
wallet2: fix duplicate output making it to the RPC
moneromooo-monero Sep 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions contrib/epee/include/storages/portable_storage_from_bin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ namespace epee
storage_entry load_storage_entry();
void read(section& sec);
void read(std::string& str);
void read(array_entry &ae);
private:
struct recursuion_limitation_guard
{
Expand Down Expand Up @@ -114,6 +115,7 @@ namespace epee
void throwable_buffer_reader::read(t_pod_type& pod_val)
{
RECURSION_LIMITATION();
static_assert(std::is_pod<t_pod_type>::value, "POD type expected");
read(&pod_val, sizeof(pod_val));
}

Expand Down Expand Up @@ -277,5 +279,11 @@ namespace epee
m_ptr+=len;
m_count -= len;
}
inline
void throwable_buffer_reader::read(array_entry &ae)
{
RECURSION_LIMITATION();
CHECK_AND_ASSERT_THROW_MES(false, "Reading array entry is not supported");
}
}
}
13 changes: 8 additions & 5 deletions contrib/epee/include/storages/portable_storage_from_json.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include "parserse_base_utils.h"
#include "file_io_utils.h"

#define EPEE_JSON_RECURSION_LIMIT_INTERNAL 100

namespace epee
{
using namespace misc_utils::parse;
Expand All @@ -44,8 +46,9 @@ namespace epee
ASSERT_MES_AND_THROW("json parse error");
}*/
template<class t_storage>
inline void run_handler(typename t_storage::hsection current_section, std::string::const_iterator& sec_buf_begin, std::string::const_iterator buf_end, t_storage& stg)
inline void run_handler(typename t_storage::hsection current_section, std::string::const_iterator& sec_buf_begin, std::string::const_iterator buf_end, t_storage& stg, unsigned int recursion)
{
CHECK_AND_ASSERT_THROW_MES(recursion < EPEE_JSON_RECURSION_LIMIT_INTERNAL, "Wrong JSON data: recursion limitation (" << EPEE_JSON_RECURSION_LIMIT_INTERNAL << ") exceeded");

std::string::const_iterator sub_element_start;
std::string name;
Expand Down Expand Up @@ -157,7 +160,7 @@ namespace epee
//sub section here
typename t_storage::hsection new_sec = stg.open_section(name, current_section, true);
CHECK_AND_ASSERT_THROW_MES(new_sec, "Failed to insert new section in json: " << std::string(it, buf_end));
run_handler(new_sec, it, buf_end, stg);
run_handler(new_sec, it, buf_end, stg, recursion + 1);
state = match_state_wonder_after_value;
}else if(*it == '[')
{//array of something
Expand Down Expand Up @@ -186,7 +189,7 @@ namespace epee
typename t_storage::hsection new_sec = nullptr;
h_array = stg.insert_first_section(name, new_sec, current_section);
CHECK_AND_ASSERT_THROW_MES(h_array&&new_sec, "failed to create new section");
run_handler(new_sec, it, buf_end, stg);
run_handler(new_sec, it, buf_end, stg, recursion + 1);
state = match_state_array_after_value;
array_md = array_mode_sections;
}else if(*it == '"')
Expand Down Expand Up @@ -260,7 +263,7 @@ namespace epee
typename t_storage::hsection new_sec = NULL;
bool res = stg.insert_next_section(h_array, new_sec);
CHECK_AND_ASSERT_THROW_MES(res&&new_sec, "failed to insert next section");
run_handler(new_sec, it, buf_end, stg);
run_handler(new_sec, it, buf_end, stg, recursion + 1);
state = match_state_array_after_value;
}else CHECK_ISSPACE();
break;
Expand Down Expand Up @@ -362,7 +365,7 @@ namespace epee
std::string::const_iterator sec_buf_begin = buff_json.begin();
try
{
run_handler(nullptr, sec_buf_begin, buff_json.end(), stg);
run_handler(nullptr, sec_buf_begin, buff_json.end(), stg, 0);
return true;
}
catch(const std::exception& ex)
Expand Down
41 changes: 33 additions & 8 deletions src/wallet/wallet2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
size_t pk_index = 0;
std::vector<tx_scan_info_t> tx_scan_info(tx.vout.size());
std::deque<bool> output_found(tx.vout.size(), false);
uint64_t total_received_1 = 0;
while (!tx.vout.empty())
{
// if tx.vout is not empty, we loop through all tx pubkeys
Expand Down Expand Up @@ -1518,6 +1519,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
+ ", m_transfers.size() is " + boost::lexical_cast<std::string>(m_transfers.size()));
if (kit == m_pub_keys.end())
{
uint64_t amount = tx.vout[o].amount ? tx.vout[o].amount : tx_scan_info[o].amount;
if (!pool)
{
m_transfers.push_back(boost::value_initialized<transfer_details>());
Expand All @@ -1530,14 +1532,13 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
td.m_key_image = tx_scan_info[o].ki;
td.m_key_image_known = !m_watch_only && !m_multisig;
td.m_key_image_partial = m_multisig;
td.m_amount = tx.vout[o].amount;
td.m_amount = amount;
td.m_pk_index = pk_index - 1;
td.m_subaddr_index = tx_scan_info[o].received->index;
expand_subaddresses(tx_scan_info[o].received->index);
if (td.m_amount == 0)
if (tx.vout[o].amount == 0)
{
td.m_mask = tx_scan_info[o].mask;
td.m_amount = tx_scan_info[o].amount;
td.m_rct = true;
}
else if (miner_tx && tx.version == 2)
Expand Down Expand Up @@ -1565,22 +1566,32 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
if (0 != m_callback)
m_callback->on_money_received(height, txid, tx, td.m_amount, td.m_subaddr_index);
}
total_received_1 += amount;
}
else if (m_transfers[kit->second].m_spent || m_transfers[kit->second].amount() >= tx_scan_info[o].amount)
{
LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first)
<< " from received " << print_money(tx_scan_info[o].amount) << " output already exists with "
<< (m_transfers[kit->second].m_spent ? "spent" : "unspent") << " "
<< print_money(m_transfers[kit->second].amount()) << " in tx " << m_transfers[kit->second].m_txid << ", received output ignored");
THROW_WALLET_EXCEPTION_IF(tx_money_got_in_outs[tx_scan_info[o].received->index] < tx_scan_info[o].amount,
error::wallet_internal_error, "Unexpected values of new and old outputs");
tx_money_got_in_outs[tx_scan_info[o].received->index] -= tx_scan_info[o].amount;
}
else
{
LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first)
<< " from received " << print_money(tx_scan_info[o].amount) << " output already exists with "
<< print_money(m_transfers[kit->second].amount()) << ", replacing with new output");
// The new larger output replaced a previous smaller one
tx_money_got_in_outs[tx_scan_info[o].received->index] -= tx_scan_info[o].amount;

THROW_WALLET_EXCEPTION_IF(tx_money_got_in_outs[tx_scan_info[o].received->index] < tx_scan_info[o].amount,
error::wallet_internal_error, "Unexpected values of new and old outputs");
THROW_WALLET_EXCEPTION_IF(m_transfers[kit->second].amount() > tx_scan_info[o].amount,
error::wallet_internal_error, "Unexpected values of new and old outputs");
tx_money_got_in_outs[tx_scan_info[o].received->index] -= m_transfers[kit->second].amount();

uint64_t amount = tx.vout[o].amount ? tx.vout[o].amount : tx_scan_info[o].amount;
uint64_t extra_amount = amount - m_transfers[kit->second].amount();
if (!pool)
{
transfer_details &td = m_transfers[kit->second];
Expand All @@ -1589,14 +1600,13 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
td.m_global_output_index = o_indices[o];
td.m_tx = (const cryptonote::transaction_prefix&)tx;
td.m_txid = txid;
td.m_amount = tx.vout[o].amount;
td.m_amount = amount;
td.m_pk_index = pk_index - 1;
td.m_subaddr_index = tx_scan_info[o].received->index;
expand_subaddresses(tx_scan_info[o].received->index);
if (td.m_amount == 0)
if (tx.vout[o].amount == 0)
{
td.m_mask = tx_scan_info[o].mask;
td.m_amount = tx_scan_info[o].amount;
td.m_rct = true;
}
else if (miner_tx && tx.version == 2)
Expand All @@ -1623,6 +1633,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
if (0 != m_callback)
m_callback->on_money_received(height, txid, tx, td.m_amount, td.m_subaddr_index);
}
total_received_1 += extra_amount;
}
}
}
Expand Down Expand Up @@ -1744,6 +1755,20 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
}
}

uint64_t total_received_2 = 0;
for (const auto& i : tx_money_got_in_outs)
total_received_2 += i.second;
if (total_received_1 != total_received_2)
{
const el::Level level = el::Level::Warning;
MCLOG_RED(level, "global", "**********************************************************************");
MCLOG_RED(level, "global", "Consistency failure in amounts received");
MCLOG_RED(level, "global", "Check transaction " << txid);
MCLOG_RED(level, "global", "**********************************************************************");
exit(1);
return;
}

for (const auto& i : tx_money_got_in_outs)
{
payment_details payment;
Expand Down