What changed, and why it matters
This commit fixes a bug in Monero's wallet software where exporting 'key images' (cryptographic proofs that help verify account balances without exposing private keys) could use an outdated cached value instead of the freshly generated correct one. The change ensures the wallet always exports the newly generated key image, and adds a test to confirm this behavior. If the wrong key image were exported, a wallet relying on it could fail to prove ownership of funds correctly, potentially causing accounting errors or denial of service in balance verification.
Review related wallet functions that mix cached and freshly generated key images to ensure consistency; consider adding explicit cache invalidation logic when key images are regenerated; include this fix in release notes as a correctness fix for wallet key image export.
Security signals we found
Use of stale cached cryptographic material instead of freshly generated value
Mismatch between signed data and exported data in a key-image export function
Added regression test demonstrating cache invalidation scenario
Evidence from the diff
In wallet2::export_key_images, the code was generating a ring signature using the freshly derived key image ki, but then appending the cached td.m_key_image to the returned vector. If the cached key image was stale or incorrect, the exported key image would not match the signature produced. The patch aligns the exported key image with the one used in signature generation by pushing ki instead of td.m_key_image. A unit test is added that deliberately corrupts the cached key image and verifies that export_key_images(false) still returns the correct generated key image and that the signature verifies.
Changed components
src/wallet/wallet2.cpptests/unit_tests/wallet_storage.cppInspect captured patch +51 / −2
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index c128eda..36402ea 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -13131,9 +13131,9 @@ std::pair<uint64_t, std::vector<std::pair<crypto::key_image, crypto::signature>>
std::vector<const crypto::public_key*> key_ptrs;
key_ptrs.push_back(&pkey);
- crypto::generate_ring_signature((const crypto::hash&)td.m_key_image, td.m_key_image, key_ptrs, in_ephemeral.sec, 0, &signature);
+ crypto::generate_ring_signature((const crypto::hash&)ki, ki, key_ptrs, in_ephemeral.sec, 0, &signature);
- ski.push_back(std::make_pair(td.m_key_image, signature));
+ ski.push_back(std::make_pair(ki, signature));
}
return std::make_pair(offset, ski);
}
diff --git a/tests/unit_tests/wallet_storage.cpp b/tests/unit_tests/wallet_storage.cpp
index 6d25686..db50ee8 100644
--- a/tests/unit_tests/wallet_storage.cpp
+++ b/tests/unit_tests/wallet_storage.cpp
@@ -44,6 +44,25 @@ static constexpr const char WALLET_00fd416a_PRIMARY_ADDRESS[] =
// https://github.com/monero-project/monero/blob/67d190ce7c33602b6a3b804f633ee1ddb7fbb4a1/src/wallet/wallet2.cpp#L156
static constexpr const char WALLET2_ASCII_OUTPUT_MAGIC[] = "MoneroAsciiDataV1";
+class wallet_accessor_test
+{
+public:
+ static void forget_cached_key_image(tools::wallet2 &wallet, const size_t index)
+ {
+ crypto::key_image stale_key_image = AUTO_VAL_INIT(stale_key_image);
+ tools::wallet2::transfer_details &td = wallet.m_transfers.at(index);
+ td.m_key_image = stale_key_image;
+ td.m_key_image_known = false;
+ td.m_key_image_request = true;
+ td.m_key_image_partial = false;
+ }
+
+ static crypto::public_key get_public_key(const tools::wallet2 &wallet, const size_t index)
+ {
+ return wallet.m_transfers.at(index).get_public_key();
+ }
+};
+
TEST(wallet_storage, store_to_file2file)
{
const path source_wallet_file = unit_test::data_dir / "wallet_00fd416a";
@@ -136,6 +155,36 @@ TEST(wallet_storage, store_to_mem2file)
EXPECT_TRUE(is_file_exist(target_wallet_file.string() + ".keys"));
}
+TEST(wallet_storage, export_key_images_uses_generated_key_image)
+{
+ const path wallet_file = unit_test::data_dir / "wallet_9svHk1";
+ epee::wipeable_string password("test");
+
+ tools::wallet2 w(cryptonote::TESTNET);
+ w.load(wallet_file.string(), password);
+ tools::wallet_keys_unlocker unlocker(w, &password);
+
+ const auto original = w.export_key_images(true);
+ ASSERT_EQ(0, original.first);
+ ASSERT_FALSE(original.second.empty());
+ const crypto::key_image expected_key_image = original.second.front().first;
+
+ wallet_accessor_test::forget_cached_key_image(w, 0);
+
+ const auto exported = w.export_key_images(false);
+ ASSERT_EQ(0, exported.first);
+ ASSERT_EQ(original.second.size(), exported.second.size());
+
+ const crypto::key_image &exported_key_image = exported.second.front().first;
+ EXPECT_TRUE(expected_key_image == exported_key_image);
+
+ const crypto::public_key pkey = wallet_accessor_test::get_public_key(w, 0);
+ std::vector<const crypto::public_key*> key_ptrs;
+ key_ptrs.push_back(&pkey);
+ EXPECT_TRUE(crypto::check_ring_signature((const crypto::hash&)exported_key_image,
+ exported_key_image, key_ptrs, &exported.second.front().second));
+}
+
TEST(wallet_storage, change_password_same_file)
{
const path source_wallet_file = unit_test::data_dir / "wallet_00fd416a";
Why this scored 44/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.