How it was found

describe_transfer wallet-rpc indexes the ring with real_output_in_tx_index instead of real_output → cold-wallet pre-sign review shows a decoy as the "real input"

Repository: monero-project/monero — src/wallet/wallet_rpc_server.cpp. Reproduced against unmodified upstream source built from /home/beni/Monero/monero-src (HEAD 082b600a2, master). The same two lines exist on release-v0.18.

Summary

describe_transfer is the JSON-RPC method a cold-wallet operator (or a GUI / payment integration wrapping monero-wallet-rpc) calls before signing an unsigned or multisig txset, to see what the transaction is actually going to do. For each input it returns a sources[i] block that includes global_index and pubkey — advertised as the real ring entry being spent.

The handler reads those two fields from the wrong slot of the ring. tx_source_entry carries two distinct numbers: real_output (the position of the real entry inside outputs, the ring) and real_output_in_tx_index (the position the output had inside its source transaction's vout). They are not the same. outputs is the ring (size = ring_size, currently 16); real_output_in_tx_index is bounded only by the source tx's vout length and is 0 for any coinbase-derived UTXO. on_describe_transfer indexes the ring with the second number, so the global_index and pubkey it shows the operator describe whichever ring slot happens to sit at position real_output_in_tx_index — usually a decoy.

The signing path is unaffected. construct_tx_with_tx_key reads the real entry from outputs[real_output] (cryptonote_tx_utils.cpp:365,373); so do multisig_tx_builder_ringct.cpp:113 and simplewallet.cpp:6213. Only this RPC mixes them up. The result is that describe_transfer and the broadcast tx disagree about which output the wallet is about to spend.

Buggy block, src/wallet/wallet_rpc_server.cpp (master 082b600a2):

1564  for (size_t s = 0; s < cd.sources.size(); ++s)
1565  {
1566    const cryptonote::tx_source_entry &src_in = cd.sources[s];
1567    wallet_rpc::COMMAND_RPC_DESCRIBE_TRANSFER::source &src_out = desc.sources.emplace_back();
1568    src_out.amount = src_in.amount;
1569    src_out.global_index = src_in.outputs.at(src_in.real_output_in_tx_index).first;
1570    src_out.rct = src_in.rct;
1571    src_out.pubkey = epee::string_tools::pod_to_hex(
1572                        src_in.outputs.at(src_in.real_output_in_tx_index).second);

The struct definition documents the meaning, and the serialiser only bounds real_output, not real_output_in_tx_index:

// src/cryptonote_core/cryptonote_tx_utils.h
45  std::vector<output_entry> outputs;     // index + key + ringct commitment (the RING)
46  uint64_t real_output;                  // index in `outputs` of the real entry
49  uint64_t real_output_in_tx_index;      // index in the SOURCE TX's vout
...
68    if (real_output >= outputs.size())   // serialiser only enforces this one
69      return false;

Other callers all use real_output:

// src/cryptonote_core/cryptonote_tx_utils.cpp:365
const auto& out_key = src_entr.outputs[src_entr.real_output].second.dest;
// src/multisig/multisig_tx_builder_ringct.cpp:113
rct::rct2pk(src.outputs[src.real_output].second.dest),
// src/simplewallet/simplewallet.cpp:6213
if (src.outputs[src.real_output].second.dest == td.get_public_key())

And wallet2::transfer_selected_rct is what populates the two fields in the first place, so you can see them being assigned to clearly different things:

// src/wallet/wallet2.cpp:9985–9992
*it_to_replace = real_oe;
src.real_out_tx_key             = get_tx_pub_key_from_extra(td.m_tx, td.m_pk_index);
src.real_out_additional_tx_keys = get_additional_tx_pub_keys_from_extra(td.m_tx);
src.real_output                 = it_to_replace - src.outputs.begin();   // ring slot
src.real_output_in_tx_index     = td.m_internal_output_index;             // source-tx vout index

There's a second, smaller failure mode falling out of the same line. Because real_output_in_tx_index isn't bounded by outputs.size(), an unsigned txset whose source UTXO came from a tx with more than ring_size outputs (batched payouts, miner consolidations) makes outputs.at(real_output_in_tx_index) throw std::out_of_range. The catch a few frames up converts that into WALLET_RPC_ERROR_CODE_BAD_UNSIGNED_TX_DATA "failed to parse unsigned transfers". The same blob signs and broadcasts cleanly via sign_transfer / submit_transfer — only the review path is broken. Operators who rely on describe_transfer to gate signing on legitimate batched txsets get a hard fail with a misleading error message.

Releases Affected

master @ 082b600a2 and the release-v0.18 branch (the two lines are byte-identical there). The describe path lands in tree well before the current top commits — this isn't a regression from a single PR. The Ubuntu monero package ships the same handler.

Steps to Reproduce

I packaged the entire repro as a self-contained bash script that drives the unmodified upstream binaries built from /home/beni/Monero/monero-src. It uses no mocks, no patches, no test harness — just a real monerod --regtest --offline and two monero-wallet-rpc instances on loopback.

Prerequisites — build once:

cd /home/beni/Monero/monero-src
mkdir -p build/release && cd build/release
cmake -D CMAKE_BUILD_TYPE=Release -D BUILD_TESTS=OFF \
      -D MONERO_WALLET_CRYPTO_LIBRARY=cn -D ARCH=native \
      -D MANUAL_SUBMODULES=1 ../..
make -j$(nproc)
# Binaries land at build/release/bin/{monerod,monero-wallet-rpc}

The script checks for those binaries at startup and exits with an error if they're missing. It also greps the buggy lines out of the source tree before doing anything else, so reviewers can confirm the binary they're about to run was compiled from those exact lines.

Run (~90 s):

bash reports/poc_describe_transfer_wrong_ring_index.sh

What it does:

1. Starts monerod --regtest --fixed-difficulty 1 --offline on loopback.
2. Spins up wallet_a (full / cold) on RPC :18190. Mines 80 blocks to it, then 60 buffer blocks elsewhere so the outputs unlock.
3. Captures wallet_a's UTXO ledger via incoming_transfers. That's the ground truth — for every output the wallet owns, we now have the (key_image, global_index, pubkey) triple.
4. Spins up wallet_b (watch-only of wallet_a) on RPC :18191 via generate_from_keys with viewkey + address only. Refreshes. Imports key images so it knows what's unspent.
5. wallet_b calls transfer with do_not_relay=true. Because it's watch-only, fill_response returns the cold-signing blob in unsigned_txset. That's the same blob a cold operator would receive in a real flow.
6. wallet_a calls describe_transfer on the unsigned txset. We capture each sources[i].global_index and sources[i].pubkey.
7. wallet_a calls sign_transfer on the same blob. wallet_b calls submit_transfer to relay it. We mine one block.
8. We pull the on-chain tx from the daemon (get_transactions, decode_as_json=true), read vin[i].k_image for each input, and look each one up in step 3's UTXO ledger. That gives the truth for every input: which global_index, which pubkey.
9. Per-input compare against describe.

Excerpted output from a real run (full log: reports/_poc_describe_transfer_run.log):

══ 0a) Show the buggy source lines compiled into the binaries above ══
    wallet_rpc_server.cpp:1569:          src_out.global_index = src_in.outputs.at(src_in.real_output_in_tx_index).first;
    wallet_rpc_server.cpp:1571:          src_out.pubkey = epee::string_tools::pod_to_hex(src_in.outputs.at(src_in.real_output_in_tx_index).second);
  [PASS] monerod    : Monero 'Fluorine Fermi' (v0.18.1.0-unknown)
  [PASS] wallet-rpc : Monero 'Fluorine Fermi' (v0.18.1.0-unknown)

══ 8) wallet_b submits the signed tx (submit_transfer) — broadcasts on-chain ══
  [PASS] tx broadcast on regtest chain: 7e1ba85c6c44d85f8aca7b82a30f1abb1b3703f4acd25096aa6b1f4b48afaf42

══ 9) Pull the on-chain tx; for EACH input, recover the real output via k_image match ══
  [PASS] On-chain tx has 2 input(s); each was matched against wallet_a's UTXO ledger:
        k_image=da3a45a17a5bf33b…  amount=35179037333548  global_index=79  pubkey=387dced4092abedf…
        k_image=bec0c27ec4c4a996…  amount=35184338534400  global_index= 0  pubkey=2a0e7912bf34820c…

══ 10) Per-input compare: describe_transfer  vs  on-chain ground truth ══
  source[0] amount=35184338534400  →  MATCHES  (real_output happened to == real_output_in_tx_index)
    describe : GI=0      dest_pubkey=2a0e7912bf34820cbfd9221f351b82aa…
    TRUTH    : GI=0           pubkey=2a0e7912bf34820cbfd9221f351b82aa…
  source[1] amount=35179037333548  →  *** MISMATCH ***   (BUG TRIGGERED)
    describe : GI=0      dest_pubkey=2a0e7912bf34820cbfd9221f351b82aa…
    TRUTH    : GI=79          pubkey=387dced4092abedf248f68ea2a2957ec…

  [PASS] *** BUG CONFIRMED *** — 1 of 2 input(s) misreported by describe_transfer

Note what describe says about source[1]: global_index=0, pubkey=2a0e7912…. That's identical to source[0]. Both sources came from coinbase txs (one output each, so real_output_in_tx_index=0 on both), so the buggy line pulls outputs[0] from each ring; describe ends up reporting two ring decoys that look almost interchangeable. The actual broadcast tx (txid=7e1ba85c…) spent global_index=79 on that input — pubkey 387dced4… — which is nowhere in the description an operator would have seen.

If you want to do it by hand instead of via the script, the four core RPC calls are:

# 1) wallet_b (watch-only) — produce the unsigned txset
TX=$(curl -s http://127.0.0.1:18191/json_rpc -d '{"jsonrpc":"2.0","id":"0","method":"transfer",
  "params":{"destinations":[{"amount":1000000000000,"address":"<DEST>"}],
            "account_index":0,"priority":1,"ring_size":11,"do_not_relay":true}}')
UTX=$(echo "$TX" | python3 -c 'import sys,json; print(json.load(sys.stdin)["result"]["unsigned_txset"])')

# 2) wallet_a (full) — describe it BEFORE signing
curl -s http://127.0.0.1:18190/json_rpc \
  -d "{\"jsonrpc\":\"2.0\",\"id\":\"0\",\"method\":\"describe_transfer\",
       \"params\":{\"unsigned_txset\":\"$UTX\"}}" \
  | jq '.result.desc[0].sources[] | {amount,global_index,pubkey}'         # ← BUGGY OUTPUT

# 3) wallet_a — sign; wallet_b — submit
SIGNED=$(curl -s http://127.0.0.1:18190/json_rpc \
  -d "{\"jsonrpc\":\"2.0\",\"id\":\"0\",\"method\":\"sign_transfer\",
       \"params\":{\"unsigned_txset\":\"$UTX\"}}" \
  | python3 -c 'import sys,json; print(json.load(sys.stdin)["result"]["signed_txset"])')
curl -s http://127.0.0.1:18191/json_rpc \
  -d "{\"jsonrpc\":\"2.0\",\"id\":\"0\",\"method\":\"submit_transfer\",
       \"params\":{\"tx_data_hex\":\"$SIGNED\"}}"

# 4) Daemon — pull the on-chain tx, read its real vin[i].k_image
curl -s http://127.0.0.1:38181/get_transactions \
  -d "{\"txs_hashes\":[\"<TX_HASH>\"],\"decode_as_json\":true}" \
  | jq '.txs[0].as_json | fromjson | .vin[].key.k_image'                  # ← TRUTH

{F5885192} {F5885193} {F5885194} {F5885195} The truth.tsv artefact the script writes makes the per-input mapping easy to read; the raw describe_transfer.json and get_transactions.json are saved next to it under /tmp/lab_describe_transfer_bug/.

Possible Solution

One-line fix — use the field whose name actually means what we want:

-        src_out.global_index = src_in.outputs.at(src_in.real_output_in_tx_index).first;
+        src_out.global_index = src_in.outputs.at(src_in.real_output).first;
         src_out.rct = src_in.rct;
-        src_out.pubkey = epee::string_tools::pod_to_hex(src_in.outputs.at(src_in.real_output_in_tx_index).second);
+        src_out.pubkey = epee::string_tools::pod_to_hex(src_in.outputs.at(src_in.real_output).second);

A couple of defence-in-depth notes while you're there:

• Add an explicit guard before the lookup. The serialiser already rejects real_output >= outputs.size(), but a future change there shouldn't be the only thing standing between this RPC and an OOB read:

  THROW_WALLET_EXCEPTION_IF(src_in.real_output >= src_in.outputs.size(),
      error::wallet_internal_error, "real_output out of bounds");
  

• It would be nice to add real_output itself to the response. Operators who keep their own UTXO ledger could then sanity-check the chosen ring slot independently, instead of having to trust global_index and pubkey blindly.
• The current test fixtures don't catch this because they tend to have real_output == real_output_in_tx_index == 0. A unit test where the two diverge — easy to construct from transfer_selected_rct output — would have caught it.

Impact

describe_transfer is the cold-wallet pre-sign review primitive. When the unsigned txset comes from a side the operator doesn't fully trust — a compromised view-only wallet, a malicious co-signer, a payment-processor integration — the operator's only defence is to call this RPC and read what it says before hitting sign_transfer. Today that defence doesn't work: the description does not describe the real input.

A hostile builder who produces the unsigned txset can put a benign-looking ring decoy in the slot the bug picks (outputs[real_output_in_tx_index]). For UTXOs derived from coinbase or any source tx whose real output sits at vout[0], that's outputs[0] of the ring — the lowest-global-index decoy after sort, which to a reviewer looks like an ordinary entry. Nothing in the rest of on_describe_transfer notices the substitution: the change-handling sanity checks at line 1590 onward only constrain change destinations, not source identity. The reviewer signs. The actual signed transaction (built from outputs[real_output], untouched) burns the real output that was hidden from the description.

The same defect quietly denies pre-sign review for legitimate batched-payout txsets where the real output's source tx had more than ring_size outputs. outputs.at(real_output_in_tx_index) throws, the catch returns WALLET_RPC_ERROR_CODE_BAD_UNSIGNED_TX_DATA "failed to parse unsigned transfers", the operator can't review it, but sign_transfer and submit_transfer accept and broadcast the same blob without complaint.

Note

The PoC script reports/poc_describe_transfer_wrong_ring_index.sh and this write-up were iterated with editor/AI help for the regtest plumbing and shell wiring. The buggy lines, the index semantics in tx_source_entry, and the on-chain ground-truth comparison were checked by reading the source and the captured RPC responses against the tree at 082b600a2. The script greps the buggy lines from the source tree at startup, so anyone re-running it can confirm the binary it's exercising was compiled from those lines.

Bounty

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

45BYNkdWvv45fovzeSgnNdMcZ8Pn9bdnegRyDcggz9XBUbrrgDhwfx63rpvesZMwWEKKQFms81v8hPbCX2eSCb3m9nFk9ZX