What the developer wrote
Authored by tobtoht
Merge pull request #11462
80f75ea wallet2: trim stale transfer maps after output imports (selsta)
ACKs: jpk68, j-berman, plowsof
What changed, and why it matters
This Monero wallet update fixes a bookkeeping bug. When a wallet imported a list of its owned outputs and that list was smaller than a previous import, internal lookup maps (key images and public keys) could still point to entries that no longer existed. The patch trims those stale pointers whenever the transfer list is shrunk, and also repairs them when loading an older wallet cache that has never been refreshed from a node. The main risk is that stale indices could cause the wallet to crash or behave incorrectly, but the code already threw an error if an out-of-range index was detected, so this is more of a robustness/correctness fix than an obvious exploit path.
Treat as a routine correctness/robustness fix. Users who import outputs (especially cold-wallet or offline signing workflows) should upgrade to avoid stale-cache errors. No urgent security response is indicated by the diff alone. If a CVE or advisory is later published, reassess.
Security signals we found
Internal index/cache consistency fix in wallet output handling
Prevents out-of-range references after transfer list resize
Adds defensive repair on wallet cache load for unrefreshed wallets
No explicit security claim made by commit message or vendor
Evidence from the diff
wallet2 maintains m_transfers (the list of owned outputs) plus two index maps: m_key_images (key_image -> index in m_transfers) and m_pub_keys (public_key -> index in m_transfers). import_outputs() can resize m_transfers down when the newly imported output set is smaller than the existing one, but previously it did not remove map entries whose stored index was now beyond the new size. The new trim_transfer_maps(num_outputs) erases such entries. A similar repair is run during load_wallet_cache() when m_has_ever_refreshed_from_node is false. The existing code already threw wallet_internal_error if a key_image index was out of range, so the patch prevents that error condition and keeps the caches consistent.
Changed components
src/wallet/wallet2.cppsrc/wallet/wallet2.hwallet2::import_outputswallet2::load_wallet_cachem_key_images cachem_pub_keys cachem_transfers vectorInspect captured patch +29 / −0
### src/wallet/wallet2.cpp
@@ -6749,6 +6749,24 @@ void wallet2::load(const std::string& wallet_, const epee::wipeable_string& pass
}
}
//----------------------------------------------------------------------------------------------------
+void wallet2::trim_transfer_maps(size_t num_transfers)
+{
+ for (auto it = m_key_images.begin(); it != m_key_images.end(); )
+ {
+ if (it->second >= num_transfers)
+ it = m_key_images.erase(it);
+ else
+ ++it;
+ }
+ for (auto it = m_pub_keys.begin(); it != m_pub_keys.end(); )
+ {
+ if (it->second >= num_transfers)
+ it = m_pub_keys.erase(it);
+ else
+ ++it;
+ }
+}
+//----------------------------------------------------------------------------------------------------
void wallet2::load_wallet_cache(const bool use_fs, const std::string& cache_buf)
{
boost::system::error_code e;
@@ -6863,6 +6881,10 @@ void wallet2::load_wallet_cache(const bool use_fs, const std::string& cache_buf)
ar >> *this;
}
}
+ // Repair stale indices from older output imports.
+ if (!m_has_ever_refreshed_from_node)
+ trim_transfer_maps(m_transfers.size());
+
for (const auto &key_image : m_key_images)
THROW_WALLET_EXCEPTION_IF(key_image.second >= m_transfers.size(), error::wallet_internal_error,
std::string("Key images cache contains illegal transfer offset: ") + std::to_string(key_image.second)
@@ -14511,7 +14533,10 @@ size_t wallet2::import_outputs(const std::tuple<uint64_t, uint64_t, std::vector<
if (offset + output_array.size() > m_transfers.size())
m_transfers.resize(offset + output_array.size());
else if (num_outputs < m_transfers.size())
+ {
+ trim_transfer_maps(num_outputs);
m_transfers.resize(num_outputs);
+ }
for (size_t i = 0; i < output_array.size(); ++i)
{
@@ -14591,7 +14616,10 @@ size_t wallet2::import_outputs(const std::tuple<uint64_t, uint64_t, std::vector<
if (offset + output_array.size() > m_transfers.size())
m_transfers.resize(offset + output_array.size());
else if (num_outputs < m_transfers.size())
+ {
+ trim_transfer_maps(num_outputs);
m_transfers.resize(num_outputs);
+ }
for (size_t i = 0; i < output_array.size(); ++i)
{
### src/wallet/wallet2.h
@@ -1555,6 +1555,7 @@ namespace tools
bool load_keys_buf(const std::string& keys_buf, const epee::wipeable_string& password);
bool load_keys_buf(const std::string& keys_buf, const epee::wipeable_string& password, boost::optional<crypto::chacha_key>& keys_to_encrypt);
void load_wallet_cache(const bool use_fs, const std::string& cache_buf = "");
+ void trim_transfer_maps(size_t num_transfers);
void process_new_transaction(const crypto::hash &txid, const cryptonote::transaction& tx, const std::vector<uint64_t> &o_indices, uint64_t height, uint8_t block_version, uint64_t ts, bool miner_tx, bool pool, bool double_spend_seen, const tx_cache_data &tx_cache_data, std::map<std::pair<uint64_t, uint64_t>, size_t> *output_tracker_cache = NULL, bool ignore_callbacks = false);
bool should_skip_block(const cryptonote::block &b, uint64_t height) const;
void process_new_blockchain_entry(const cryptonote::block& b, const cryptonote::block_complete_entry& bche, const parsed_block &parsed_block, const crypto::hash& bl_id, uint64_t height, const std::vector<tx_cache_data> &tx_cache_data, size_t tx_cache_data_offset, std::map<std::pair<uint64_t, uint64_t>, size_t> *output_tracker_cache = NULL);Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.