wallet2: validate cached transfer indices
What the developer wrote
Authored by selsta
wallet2: validate cached transfer indices
Reported by hacksandhops
What changed, and why it matters
This update adds safety checks to the Monero wallet software to detect and stop use of corrupted or tampered wallet cache data. Specifically, it verifies that internal indexes pointing to past transfers are not larger than the actual list of transfers. Without these checks, a damaged or maliciously crafted wallet cache could cause the wallet to read from invalid memory locations, potentially leading to crashes or unpredictable behavior. The change is defensive hardening rather than a fix for an active remote attack.
Treat as a defensive security hardening patch. Users should upgrade wallet software and avoid opening wallet cache files from untrusted sources. Developers should consider whether additional cache validation (e.g., checksums, version checks) is warranted. No immediate emergency response is indicated unless further evidence shows the issue is remotely exploitable.
Security signals we found
Out-of-bounds access prevention on cached transfer indices
Defensive validation of wallet cache integrity at load time
Addition of THROW_WALLET_EXCEPTION_IF bounds checks in three wallet operations
Reporter credited as 'hacksandhops' in commit message
Evidence from the diff
The patch adds bounds checks in three places in wallet2.cpp where cached key-image-to-transfer-index mappings (m_key_images) and public-key-to-transfer-index mappings (m_pub_keys) are dereferenced against m_transfers. Previously, if a cache entry contained an offset >= m_transfers.size(), the code would perform an out-of-bounds read/access. The checks now throw a wallet_internal_error exception instead. The checks are added in process_new_transaction, load_wallet_cache, and get_spend_proof. This protects against use of an inconsistent or corrupted wallet cache file.
Changed components
src/wallet/wallet2.cppwallet cache loading (load_wallet_cache)incoming transaction processing (process_new_transaction)spend proof generation (get_spend_proof)Inspect captured patch +14 / −0
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 834679f..26226c5 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -2732,6 +2732,9 @@ void wallet2::process_new_transaction(const crypto::hash &txid, const cryptonote
auto it = m_key_images.find(in_to_key.k_image);
if(it != m_key_images.end())
{
+ THROW_WALLET_EXCEPTION_IF(it->second >= m_transfers.size(), error::wallet_internal_error,
+ std::string("Key images cache contains illegal transfer offset: ") + std::to_string(it->second)
+ + " m_transfers.size() = " + std::to_string(m_transfers.size()));
transfer_details& td = m_transfers[it->second];
uint64_t amount = in_to_key.amount;
if (amount > 0)
@@ -6689,6 +6692,14 @@ void wallet2::load_wallet_cache(const bool use_fs, const std::string& cache_buf)
ar >> *this;
}
}
+ 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)
+ + " m_transfers.size() = " + std::to_string(m_transfers.size()));
+ for (const auto &pub_key : m_pub_keys)
+ THROW_WALLET_EXCEPTION_IF(pub_key.second >= m_transfers.size(), error::wallet_internal_error,
+ std::string("Public keys cache contains illegal transfer offset: ") + std::to_string(pub_key.second)
+ + " m_transfers.size() = " + std::to_string(m_transfers.size()));
THROW_WALLET_EXCEPTION_IF(
m_account_public_address.m_spend_public_key != m_account.get_keys().m_account_address.m_spend_public_key ||
m_account_public_address.m_view_public_key != m_account.get_keys().m_account_address.m_view_public_key,
@@ -11799,6 +11810,9 @@ std::string wallet2::get_spend_proof(const crypto::hash &txid, const std::string
}
// derive the real output keypair
+ THROW_WALLET_EXCEPTION_IF(found->second >= m_transfers.size(), error::wallet_internal_error,
+ std::string("Key images cache contains illegal transfer offset: ") + std::to_string(found->second)
+ + " m_transfers.size() = " + std::to_string(m_transfers.size()));
const transfer_details& in_td = m_transfers[found->second];
crypto::public_key in_tx_out_pkey = in_td.get_public_key();
const crypto::public_key in_tx_pub_key = get_tx_pub_key_from_extra(in_td.m_tx, in_td.m_pk_index);
Why this scored 59/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.