From c80f4d416d2a392dbf7c61b500672a6156b4b77d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 7 Nov 2016 18:50:05 +0000 Subject: [PATCH] wallet: fix output collision detection for view wallets View wallets do not have the spend secret key, and are thus unable to derive key images for incoming outputs. Moreover, a previous patch set key images to zero as a means to mark an output as having an unknown key image, so they could be filled in when importing key images at a later time. That later patch caused spurious collisions. We now use public keys to detect duplicate outputs. Public keys obtained from the blockchain are checked to be identical to the ones derived locally, so can't be spoofed. --- src/crypto/crypto.h | 2 +- src/wallet/wallet2.cpp | 42 ++++++++++++++++++++++++------------------ src/wallet/wallet2.h | 24 ++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h index b396fc7db..9ca835d9e 100644 --- a/src/crypto/crypto.h +++ b/src/crypto/crypto.h @@ -236,6 +236,6 @@ namespace crypto { } } -CRYPTO_MAKE_COMPARABLE(public_key) +CRYPTO_MAKE_HASHABLE(public_key) CRYPTO_MAKE_HASHABLE(key_image) CRYPTO_MAKE_COMPARABLE(signature) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index f57a8d2ca..06e12ebfe 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -250,8 +250,6 @@ bool wallet2::wallet_generate_key_image_helper(const cryptonote::account_keys& a { if (!cryptonote::generate_key_image_helper(ack, tx_public_key, real_output_index, in_ephemeral, ki)) return false; - if (m_watch_only) - memset(&ki, 0, 32); return true; } //---------------------------------------------------------------------------------------------------- @@ -484,12 +482,12 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s THROW_WALLET_EXCEPTION_IF(tx.vout.size() <= o, error::wallet_internal_error, "wrong out in transaction: internal index=" + std::to_string(o) + ", total_outs=" + std::to_string(tx.vout.size())); - auto kit = m_key_images.find(ki[o]); - THROW_WALLET_EXCEPTION_IF(kit != m_key_images.end() && kit->second >= m_transfers.size(), - error::wallet_internal_error, std::string("Unexpected transfer index from key image: ") - + "got " + (kit == m_key_images.end() ? "" : boost::lexical_cast(kit->second)) + auto kit = m_pub_keys.find(in_ephemeral[o].pub); + THROW_WALLET_EXCEPTION_IF(kit != m_pub_keys.end() && kit->second >= m_transfers.size(), + error::wallet_internal_error, std::string("Unexpected transfer index from public key: ") + + "got " + (kit == m_pub_keys.end() ? "" : boost::lexical_cast(kit->second)) + ", m_transfers.size() is " + boost::lexical_cast(m_transfers.size())); - if (kit == m_key_images.end()) + if (kit == m_pub_keys.end()) { if (!pool) { @@ -501,6 +499,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s td.m_tx = (const cryptonote::transaction_prefix&)tx; td.m_txid = txid(); td.m_key_image = ki[o]; + td.m_key_image_known = !m_watch_only; td.m_amount = tx.vout[o].amount; if (td.m_amount == 0) { @@ -520,6 +519,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s } set_unspent(m_transfers.size()-1); m_key_images[td.m_key_image] = m_transfers.size()-1; + m_pub_keys[in_ephemeral[o].pub] = m_transfers.size()-1; LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid()); if (0 != m_callback) m_callback->on_money_received(height, tx, td.m_amount); @@ -527,14 +527,14 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s } else if (m_transfers[kit->second].m_spent || m_transfers[kit->second].amount() >= tx.vout[o].amount) { - LOG_ERROR("key image " << epee::string_tools::pod_to_hex(kit->first) + LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first) << " from received " << print_money(tx.vout[o].amount) << " output already exists with " << (m_transfers[kit->second].m_spent ? "spent" : "unspent") << " " << print_money(m_transfers[kit->second].amount()) << ", received output ignored"); } else { - LOG_ERROR("key image " << epee::string_tools::pod_to_hex(kit->first) + LOG_ERROR("Public key " << epee::string_tools::pod_to_hex(kit->first) << " from received " << print_money(tx.vout[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 @@ -565,7 +565,7 @@ void wallet2::process_new_transaction(const cryptonote::transaction& tx, const s td.m_mask = rct::identity(); td.m_rct = false; } - THROW_WALLET_EXCEPTION_IF(td.m_key_image != ki[o], error::wallet_internal_error, "Inconsistent key images"); + THROW_WALLET_EXCEPTION_IF(td.get_public_key() != in_ephemeral[o].pub, error::wallet_internal_error, "Inconsistent public keys"); THROW_WALLET_EXCEPTION_IF(td.m_spent, error::wallet_internal_error, "Inconsistent spent status"); LOG_PRINT_L0("Received money: " << print_money(td.amount()) << ", with tx: " << txid()); @@ -1345,7 +1345,13 @@ void wallet2::detach_blockchain(uint64_t height) auto it_ki = m_key_images.find(m_transfers[i].m_key_image); THROW_WALLET_EXCEPTION_IF(it_ki == m_key_images.end(), error::wallet_internal_error, "key image not found"); m_key_images.erase(it_ki); - ++transfers_detached; + } + + for(size_t i = i_start; i!= m_transfers.size();i++) + { + auto it_pk = m_pub_keys.find(m_transfers[i].get_public_key()); + THROW_WALLET_EXCEPTION_IF(it_pk == m_pub_keys.end(), error::wallet_internal_error, "public key not found"); + m_pub_keys.erase(it_pk); } m_transfers.erase(it, m_transfers.end()); @@ -1382,6 +1388,7 @@ bool wallet2::clear() m_blockchain.clear(); m_transfers.clear(); m_key_images.clear(); + m_pub_keys.clear(); m_unconfirmed_txs.clear(); m_payments.clear(); m_tx_keys.clear(); @@ -2158,13 +2165,11 @@ void wallet2::rescan_spent() std::to_string(daemon_resp.spent_status.size()) + ", expected " + std::to_string(key_images.size())); // update spent status - key_image zero_ki; - memset(&zero_ki, 0, 32); for (size_t i = 0; i < m_transfers.size(); ++i) { transfer_details& td = m_transfers[i]; // a view wallet may not know about key images - if (td.m_key_image == zero_ki) + if (!td.m_key_image_known) continue; if (td.m_spent != (daemon_resp.spent_status[i] != COMMAND_RPC_IS_KEY_IMAGE_SPENT::UNSPENT)) { @@ -4276,10 +4281,7 @@ std::vector> wallet2::export_key cryptonote::keypair in_ephemeral; cryptonote::generate_key_image_helper(m_account.get_keys(), tx_pub_key, td.m_internal_output_index, in_ephemeral, ki); - bool zero_key_image = true; - for (size_t i = 0; i < sizeof(td.m_key_image); ++i) - zero_key_image &= (td.m_key_image.data[i] == 0); - THROW_WALLET_EXCEPTION_IF(!zero_key_image && ki != td.m_key_image, + THROW_WALLET_EXCEPTION_IF(td.m_key_image_known && ki != td.m_key_image, error::wallet_internal_error, "key_image generated not matched with cached key image"); THROW_WALLET_EXCEPTION_IF(in_ephemeral.pub != pkey, error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key"); @@ -4335,7 +4337,10 @@ uint64_t wallet2::import_key_images(const std::vector(td.m_tx.vout[td.m_internal_output_index].target).key, error::wallet_internal_error, "key_image generated ephemeral public key not matched with output_key at index " + i); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index b34db0b6c..2b69dc64a 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -109,9 +109,11 @@ namespace tools rct::key m_mask; uint64_t m_amount; bool m_rct; + bool m_key_image_known; bool is_rct() const { return m_rct; } uint64_t amount() const { return m_amount; } + const crypto::public_key &get_public_key() const { return boost::get(m_tx.vout[m_internal_output_index].target).key; } }; struct payment_details @@ -409,6 +411,19 @@ namespace tools a & m_unconfirmed_payments; if(ver < 14) return; + if(ver < 15) + { + // we're loading an older wallet without a pubkey map, rebuild it + for (size_t i = 0; i < m_transfers.size(); ++i) + { + const transfer_details &td = m_transfers[i]; + const cryptonote::tx_out &out = td.m_tx.vout[td.m_internal_output_index]; + const cryptonote::txout_to_key &o = boost::get(out.target); + m_pub_keys.emplace(o.key, i); + } + return; + } + a & m_pub_keys; } /*! @@ -543,6 +558,7 @@ namespace tools transfer_container m_transfers; payment_container m_payments; std::unordered_map m_key_images; + std::unordered_map m_pub_keys; cryptonote::account_public_address m_account_public_address; std::unordered_map m_tx_notes; uint64_t m_upper_transaction_size_limit; //TODO: auto-calc this value or request from daemon, now use some fixed value @@ -567,8 +583,8 @@ namespace tools bool m_confirm_missing_payment_id; }; } -BOOST_CLASS_VERSION(tools::wallet2, 14) -BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 4) +BOOST_CLASS_VERSION(tools::wallet2, 15) +BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 5) BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1) BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 6) BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 3) @@ -597,6 +613,7 @@ namespace boost { x.m_rct = x.m_tx.vout[x.m_internal_output_index].amount == 0; } + x.m_key_image_known = true; } template @@ -644,6 +661,9 @@ namespace boost return; } a & x.m_rct; + if (ver < 5) + return; + a & x.m_key_image_known; } template