Skip to content

Commit

Permalink
DoS/RPC fixes
Browse files Browse the repository at this point in the history 
monero-project/monero#4438
  • Loading branch information
@thaerkh
thaerkh committed Sep 25, 2018
1 parent 1833610 commit 967edcb
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 13 deletions.
8 changes: 8 additions & 0 deletions contrib/epee/include/storages/portable_storage_from_bin.h
View file
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1123,6 +1123,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 @@ -1268,6 +1269,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 @@ -1280,14 +1282,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 @@ -1315,22 +1316,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 @@ -1339,14 +1350,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 @@ -1373,6 +1383,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 @@ -1496,6 +1507,20 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
LOG_PRINT_L2("Found unencrypted payment ID: " << payment_id);
}

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

1 comment on commit 967edcb

@LordMajestros
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the fixes for the monero burning bug?

Please sign in to comment.