Reports
MEDIA[APIS]#3723315

Monero: describe_transfer de wallet-rpc muestra un señuelo del anillo como entrada real antes de firmar en frío

El método describe_transfer de monero-wallet-rpc tomaba la entrada del anillo con el índice equivocado, así que la revisión previa a la firma en frío podía enseñar un señuelo en lugar de la salida que de verdad se gastaba.

Resumen
Resumen en castellano de un reporte público, no una traducción literal. El código y los comandos se mantienen como en el original.

Resumen

En Monero, quien guarda los fondos en una cartera fría (sin conexión) recibe de una cartera de solo lectura, de un cofirmante multisig o de una integración de pagos un conjunto de transacciones sin firmar (unsigned_txset). Antes de firmarlo, lo normal es llamar al método JSON-RPC describe_transfer de monero-wallet-rpc, que devuelve para cada entrada un bloque sources[i] con el global_index y la pubkey de la salida que se va a gastar. Es, en la práctica, la única forma de saber qué se está firmando.

El problema estaba en on_describe_transfer (src/wallet/wallet_rpc_server.cpp). La estructura tx_source_entry guarda dos índices distintos:

  • real_output: la posición de la salida real dentro de outputs, que es el anillo (16 miembros con el tamaño actual).
  • real_output_in_tx_index: la posición que esa salida tenía dentro del vout de su transacción de origen. Vale 0, por ejemplo, en cualquier salida que venga de una transacción coinbase.

El manejador usaba el segundo para indexar el anillo:

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);

Así, lo que se le enseñaba al operador era el miembro del anillo que casualmente ocupaba la posición real_output_in_tx_index, que normalmente es un señuelo. El resto del código hace lo correcto: construct_tx_with_tx_key (cryptonote_tx_utils.cpp:365), multisig_tx_builder_ringct.cpp:113 y simplewallet.cpp:6213 leen outputs[real_output]. La firma no se ve afectada; lo que falla es solo la descripción, que acaba en desacuerdo con la transacción que se emite.

El origen de ambos valores se ve en wallet2::transfer_selected_rct, donde se asignan a cosas claramente diferentes:

// src/wallet/wallet2.cpp:9985–9992
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

Hay un segundo efecto del mismo error. El serializador solo comprueba que real_output sea menor que outputs.size(); real_output_in_tx_index no tiene ese límite. Si la salida real procede de una transacción con más salidas que el tamaño del anillo (pagos en lote, consolidaciones de minería), outputs.at(real_output_in_tx_index) lanza std::out_of_range y la RPC responde WALLET_RPC_ERROR_CODE_BAD_UNSIGNED_TX_DATA "failed to parse unsigned transfers", aunque ese mismo blob se firma y se emite sin problema con sign_transfer y submit_transfer.

Según el investigador, el fallo estaba en master (commit 082b600a2) y en la rama release-v0.18, con las dos líneas idénticas, y no venía de un cambio reciente. HackerOne lo clasifica como "Array Index Underflow".

Pasos de reproducción

El investigador lo reprodujo con binarios compilados desde el código fuente original sin modificar, sobre una red regtest local y dos instancias de monero-wallet-rpc. Compilación:

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)

Escenario completo:

  1. Arrancar monerod --regtest --fixed-difficulty 1 --offline en local.
  2. Crear wallet_a (cartera completa, la "fría") con RPC en el puerto 18190, minar 80 bloques a su favor y otros 60 a otra dirección para que las salidas se desbloqueen.
  3. Guardar como referencia el inventario de salidas de wallet_a con incoming_transfers: para cada una, la terna (key_image, global_index, pubkey).
  4. Crear wallet_b como cartera de solo lectura de wallet_a (puerto 18191) con generate_from_keys usando solo la dirección y la clave de visualización, refrescarla e importar las key images.
  5. Desde wallet_b, llamar a transfer con do_not_relay=true. Al ser de solo lectura, devuelve el blob para firma en frío en unsigned_txset.
  6. Desde wallet_a, llamar a describe_transfer con ese blob y anotar sources[i].global_index y sources[i].pubkey.
  7. Firmar con sign_transfer en wallet_a, emitir con submit_transfer desde wallet_b y minar un bloque.
  8. Pedir al nodo la transacción ya en cadena (get_transactions con decode_as_json=true), leer vin[i].k_image de cada entrada y buscarla en el inventario del paso 3 para saber qué salida se gastó realmente.
  9. Comparar entrada por entrada con lo que dijo describe_transfer.

Las llamadas esenciales, hechas a mano:

# 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

Resultado de la ejecución del investigador, con una transacción de dos entradas:

  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…

Las dos entradas venían de transacciones coinbase con una sola salida (real_output_in_tx_index=0 en ambas), así que describe_transfer devolvió outputs[0] de cada anillo: la misma salida global_index=0 para las dos. En realidad, la segunda entrada gastó global_index=79, que no aparecía en ningún sitio de la descripción.

Impacto

describe_transfer es la herramienta con la que el operador de una cartera fría comprueba lo que va a firmar cuando el blob viene de alguien en quien no confía del todo: una cartera de solo lectura comprometida, un cofirmante malicioso o una integración de pagos. Con este fallo, esa comprobación no describía la entrada real.

Quien construye el blob sin firmar puede colocar un señuelo de aspecto inocente en la posición que el error iba a leer. En salidas de coinbase o cualquier salida que esté en vout[0] de su transacción, esa posición es outputs[0], el señuelo con el global_index más bajo tras ordenar el anillo, que a ojos del revisor parece una entrada normal. Las comprobaciones posteriores de on_describe_transfer solo validan los destinos de cambio, no qué salidas se gastan. El operador firma, y la transacción, construida a partir de outputs[real_output], gasta una salida que nunca se le enseñó.

Además, en conjuntos legítimos cuyas salidas proceden de transacciones con más salidas que el tamaño del anillo, la revisión era imposible: la RPC fallaba con un mensaje engañoso de datos no válidos mientras la firma y la emisión del mismo blob funcionaban.

Remediación

El reporte figura como resuelto en HackerOne. La corrección que proponía el investigador es usar el índice correcto:

-        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);

También sugería, como defensa adicional, comprobar explícitamente el límite antes de leer, sin depender solo del serializador:

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

Además proponía incluir real_output en la respuesta para que el operador pueda contrastarlo con su propio inventario, y añadir una prueba unitaria en la que real_output y real_output_in_tx_index sean distintos: las pruebas existentes no lo detectaban porque solían tener ambos a 0.

Qué aprender de este caso

  • Cuando una estructura tiene dos índices con nombres parecidos (real_output y real_output_in_tx_index), revisa cada acceso a un array y confirma que el índice se refiere a ese array y no a otro; aquí uno indexaba el anillo y el otro el vout de otra transacción.
  • Compara el código que solo muestra con el que realmente ejecuta: si la ruta de firma usa outputs[real_output] y la de descripción usa otra cosa, la vista previa puede mentir sin que nada falle. Las funciones de previsualización o resumen de cualquier sistema de firma son un buen sitio para buscar fallos parecidos.
  • Las pruebas con datos en los que los valores coinciden por casualidad (ambos índices a 0) esconden este tipo de errores: los casos de prueba deben forzar que los campos que podrían confundirse tengan valores distintos.
  • Si una pantalla de revisión es la defensa ante un emisor no confiable, debe calcular lo que muestra a partir de los mismos datos que se firman, y un fallo al describir (como el std::out_of_range) no debería presentarse como "datos no válidos" cuando la firma sí los acepta.