What the developer wrote
Authored by tobtoht
Merge pull request #11244
bf54d18 wallet2: validate scan_tx daemon responses (Samy)
ACKs: jpk68, selsta
What changed, and why it matters
This update adds a safety check in the Monero wallet software. When the wallet asks a network node (daemon) for details about specific transactions, it now verifies that the transactions it gets back are actually the ones it asked for. Without this check, a malicious or compromised node could potentially return the wrong transactions, which might confuse the wallet about balances or transaction history. The fix makes the wallet reject any response where the returned transaction IDs do not match the requested ones.
Treat this as a security hardening fix and include it in the next wallet release. Users running wallets against untrusted or third-party daemons should upgrade. No immediate emergency response is indicated because the patch is small and defensive, but downstream packagers should apply it promptly.
Security signals we found
Missing input validation on daemon RPC response
Trust boundary crossing between wallet client and daemon
Potential transaction substitution / response mismatch
Defensive validation added for returned transaction hashes
Evidence from the diff
In wallet2::get_tx_entries(), the wallet builds a request of transaction hashes and sends it to the daemon via scan_tx. Previously, the code only verified that the number of returned transactions matched the number requested. This patch records the iterator to the first requested txid (expected_txid_begin), then after receiving res.txs, iterates the response and checks that the computed tx_hash of each returned pruned transaction equals *expected_txid, incrementing expected_txid for each entry. A mismatch now throws a wallet_internal_error. This closes a trust boundary gap where a daemon could substitute, omit, or reorder transactions in its response without detection by the wallet client.
Changed components
src/wallet/wallet2.cppwallet2::get_tx_entries()scan_tx daemon RPC handlingInspect captured patch +4 / −0
### src/wallet/wallet2.cpp
@@ -1772,6 +1772,7 @@ wallet2::tx_entry_data wallet2::get_tx_entries(const std::unordered_set<crypto::
req.prune = true;
size_t ntxes = slice + SLICE_SIZE > txids.size() ? txids.size() - slice : SLICE_SIZE;
+ const auto expected_txid_begin = it;
for (size_t i = slice; i < slice + ntxes; ++i)
{
req.txs_hashes.push_back(epee::string_tools::pod_to_hex(*it));
@@ -1785,6 +1786,7 @@ wallet2::tx_entry_data wallet2::get_tx_entries(const std::unordered_set<crypto::
THROW_WALLET_EXCEPTION_IF(res.txs.size() != req.txs_hashes.size(), error::wallet_internal_error, "Failed to get transaction from daemon");
}
+ auto expected_txid = expected_txid_begin;
for (auto& tx_info : res.txs)
{
if (!tx_info.in_pool)
@@ -1796,6 +1798,8 @@ wallet2::tx_entry_data wallet2::get_tx_entries(const std::unordered_set<crypto::
cryptonote::transaction tx;
crypto::hash tx_hash;
THROW_WALLET_EXCEPTION_IF(!get_pruned_tx(tx_info, tx, tx_hash), error::wallet_internal_error, "Failed to get transaction from daemon");
+ THROW_WALLET_EXCEPTION_IF(tx_hash != *expected_txid, error::wallet_internal_error, "Failed to get the right transaction from daemon");
+ ++expected_txid;
tx_entries.tx_entries.emplace_back(process_tx_entry_t{ std::move(tx_info), std::move(tx), std::move(tx_hash) });
}
}Why this scored 61/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.