Merge pull request #4437

3caa4038 wallet2: fix duplicate output making it to the RPC (moneromooo-monero)
58ee1f2b epee: fix invalid memory write reading an array entry (moneromooo-monero)
f61cd544 epee: fix stack overflow on crafted input (moneromooo-monero)
This commit is contained in:
Riccardo Spagni 2018-09-25 13:57:50 +02:00
commit c6ce8235d8
No known key found for this signature in database
GPG key ID: 55432DF31CCD4FCD
3 changed files with 48 additions and 12 deletions

View file

@ -59,6 +59,7 @@ namespace epee
storage_entry load_storage_entry(); storage_entry load_storage_entry();
void read(section& sec); void read(section& sec);
void read(std::string& str); void read(std::string& str);
void read(array_entry &ae);
private: private:
struct recursuion_limitation_guard struct recursuion_limitation_guard
{ {
@ -114,6 +115,7 @@ namespace epee
void throwable_buffer_reader::read(t_pod_type& pod_val) void throwable_buffer_reader::read(t_pod_type& pod_val)
{ {
RECURSION_LIMITATION(); RECURSION_LIMITATION();
static_assert(std::is_pod<t_pod_type>::value, "POD type expected");
read(&pod_val, sizeof(pod_val)); read(&pod_val, sizeof(pod_val));
} }
@ -277,5 +279,11 @@ namespace epee
m_ptr+=len; m_ptr+=len;
m_count -= 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");
}
} }
} }

View file

@ -30,6 +30,8 @@
#include "parserse_base_utils.h" #include "parserse_base_utils.h"
#include "file_io_utils.h" #include "file_io_utils.h"
#define EPEE_JSON_RECURSION_LIMIT_INTERNAL 100
namespace epee namespace epee
{ {
using namespace misc_utils::parse; using namespace misc_utils::parse;
@ -44,8 +46,9 @@ namespace epee
ASSERT_MES_AND_THROW("json parse error"); ASSERT_MES_AND_THROW("json parse error");
}*/ }*/
template<class t_storage> 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::const_iterator sub_element_start;
std::string name; std::string name;
@ -157,7 +160,7 @@ namespace epee
//sub section here //sub section here
typename t_storage::hsection new_sec = stg.open_section(name, current_section, true); 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)); 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; state = match_state_wonder_after_value;
}else if(*it == '[') }else if(*it == '[')
{//array of something {//array of something
@ -186,7 +189,7 @@ namespace epee
typename t_storage::hsection new_sec = nullptr; typename t_storage::hsection new_sec = nullptr;
h_array = stg.insert_first_section(name, new_sec, current_section); 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"); 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; state = match_state_array_after_value;
array_md = array_mode_sections; array_md = array_mode_sections;
}else if(*it == '"') }else if(*it == '"')
@ -260,7 +263,7 @@ namespace epee
typename t_storage::hsection new_sec = NULL; typename t_storage::hsection new_sec = NULL;
bool res = stg.insert_next_section(h_array, new_sec); bool res = stg.insert_next_section(h_array, new_sec);
CHECK_AND_ASSERT_THROW_MES(res&&new_sec, "failed to insert next section"); 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; state = match_state_array_after_value;
}else CHECK_ISSPACE(); }else CHECK_ISSPACE();
break; break;
@ -362,7 +365,7 @@ namespace epee
std::string::const_iterator sec_buf_begin = buff_json.begin(); std::string::const_iterator sec_buf_begin = buff_json.begin();
try try
{ {
run_handler(nullptr, sec_buf_begin, buff_json.end(), stg); run_handler(nullptr, sec_buf_begin, buff_json.end(), stg, 0);
return true; return true;
} }
catch(const std::exception& ex) catch(const std::exception& ex)

View file

@ -1121,6 +1121,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
size_t pk_index = 0; size_t pk_index = 0;
std::vector<tx_scan_info_t> tx_scan_info(tx.vout.size()); std::vector<tx_scan_info_t> tx_scan_info(tx.vout.size());
std::deque<bool> output_found(tx.vout.size(), false); std::deque<bool> output_found(tx.vout.size(), false);
uint64_t total_received_1 = 0;
while (!tx.vout.empty()) while (!tx.vout.empty())
{ {
// if tx.vout is not empty, we loop through all tx pubkeys // if tx.vout is not empty, we loop through all tx pubkeys
@ -1266,6 +1267,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
+ ", m_transfers.size() is " + boost::lexical_cast<std::string>(m_transfers.size())); + ", m_transfers.size() is " + boost::lexical_cast<std::string>(m_transfers.size()));
if (kit == m_pub_keys.end()) if (kit == m_pub_keys.end())
{ {
uint64_t amount = tx.vout[o].amount ? tx.vout[o].amount : tx_scan_info[o].amount;
if (!pool) if (!pool)
{ {
m_transfers.push_back(boost::value_initialized<transfer_details>()); m_transfers.push_back(boost::value_initialized<transfer_details>());
@ -1278,14 +1280,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 = tx_scan_info[o].ki;
td.m_key_image_known = !m_watch_only && !m_multisig; td.m_key_image_known = !m_watch_only && !m_multisig;
td.m_key_image_partial = 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_pk_index = pk_index - 1;
td.m_subaddr_index = tx_scan_info[o].received->index; td.m_subaddr_index = tx_scan_info[o].received->index;
expand_subaddresses(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_mask = tx_scan_info[o].mask;
td.m_amount = tx_scan_info[o].amount;
td.m_rct = true; td.m_rct = true;
} }
else if (miner_tx && tx.version == 2) else if (miner_tx && tx.version == 2)
@ -1313,6 +1314,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
if (0 != m_callback) if (0 != m_callback)
m_callback->on_money_received(height, txid, tx, td.m_amount, td.m_subaddr_index); 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) else if (m_transfers[kit->second].m_spent || m_transfers[kit->second].amount() >= tx_scan_info[o].amount)
{ {
@ -1320,6 +1322,9 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
<< " from received " << print_money(tx_scan_info[o].amount) << " output already exists with " << " from received " << print_money(tx_scan_info[o].amount) << " output already exists with "
<< (m_transfers[kit->second].m_spent ? "spent" : "unspent") << " " << (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"); << 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 else
{ {
@ -1327,8 +1332,14 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
<< " from received " << print_money(tx_scan_info[o].amount) << " output already exists with " << " from received " << print_money(tx_scan_info[o].amount) << " output already exists with "
<< print_money(m_transfers[kit->second].amount()) << ", replacing with new output"); << print_money(m_transfers[kit->second].amount()) << ", replacing with new output");
// The new larger output replaced a previous smaller one // 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) if (!pool)
{ {
transfer_details &td = m_transfers[kit->second]; transfer_details &td = m_transfers[kit->second];
@ -1337,14 +1348,13 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
td.m_global_output_index = o_indices[o]; td.m_global_output_index = o_indices[o];
td.m_tx = (const cryptonote::transaction_prefix&)tx; td.m_tx = (const cryptonote::transaction_prefix&)tx;
td.m_txid = txid; td.m_txid = txid;
td.m_amount = tx.vout[o].amount; td.m_amount = amount;
td.m_pk_index = pk_index - 1; td.m_pk_index = pk_index - 1;
td.m_subaddr_index = tx_scan_info[o].received->index; td.m_subaddr_index = tx_scan_info[o].received->index;
expand_subaddresses(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_mask = tx_scan_info[o].mask;
td.m_amount = tx_scan_info[o].amount;
td.m_rct = true; td.m_rct = true;
} }
else if (miner_tx && tx.version == 2) else if (miner_tx && tx.version == 2)
@ -1371,6 +1381,7 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
if (0 != m_callback) if (0 != m_callback)
m_callback->on_money_received(height, txid, tx, td.m_amount, td.m_subaddr_index); m_callback->on_money_received(height, txid, tx, td.m_amount, td.m_subaddr_index);
} }
total_received_1 += extra_amount;
} }
} }
} }
@ -1494,6 +1505,20 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
LOG_PRINT_L2("Found unencrypted payment ID: " << payment_id); 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) for (const auto& i : tx_money_got_in_outs)
{ {
payment_details payment; payment_details payment;