wallet2: filter unchecked RPC error statuses
What the developer wrote
Authored by selsta
wallet2: filter unchecked RPC error statuses
What changed, and why it matters
This commit changes how the Monero wallet handles error responses from the daemon (the network node it talks to). Previously, certain RPC calls used a macro that did not check the daemon's returned status code in all cases. The new code explicitly checks that status and throws a proper wallet error if the daemon reports a failure. In practical terms, this reduces the chance that the wallet will silently continue with stale or incomplete data when the daemon is actually telling it something went wrong. It is a hardening fix rather than a clear-cut exploit patch, and the commit message does not describe a specific vulnerability.
Treat as a defensive security hardening commit. Review whether THROW_ON_RPC_RESPONSE_ERROR_GENERIC is still used elsewhere and whether those remaining sites should also be converted. Consider adding regression tests that simulate daemon RPC error statuses for these endpoints to ensure the wallet now throws instead of proceeding.
Security signals we found
Unchecked RPC status field previously could allow processing of error responses
Replacement of generic error macro with status-aware error macro
Daemon trust setting (m_trusted_daemon) now influences status interpretation
Hardcoded endpoint strings remain unchanged; only error handling changes
Evidence from the diff
The patch replaces THROW_ON_RPC_RESPONSE_ERROR_GENERIC with THROW_ON_RPC_RESPONSE_ERROR in five wallet2.cpp RPC call sites: /get_output_distribution.bin, /gettransactions (three call sites), and /get_public_nodes. The generic macro appears to throw only on HTTP/connection errors, while THROW_ON_RPC_RESPONSE_ERROR additionally evaluates res.status via get_rpc_status(m_trusted_daemon, res.status) and maps it to error::wallet_generic_rpc_error. This ensures daemon-reported RPC failures are propagated as exceptions rather than potentially being ignored. The change is defensive: it prevents the wallet from acting on responses whose status field indicates an error.
Changed components
src/wallet/wallet2.cppwallet2::get_rct_distributionwallet2::set_tx_keywallet2::get_spend_proofwallet2::check_spend_proofwallet2::get_public_nodesInspect captured patch +5 / −5
### src/wallet/wallet2.cpp
@@ -4345,7 +4345,7 @@ bool wallet2::get_rct_distribution(uint64_t &start_height, std::vector<uint64_t>
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
r = net_utils::invoke_http_bin("/get_output_distribution.bin", req, res, *m_http_client, rpc_timeout);
- THROW_ON_RPC_RESPONSE_ERROR_GENERIC(r, {}, res, "/get_output_distribution.bin");
+ THROW_ON_RPC_RESPONSE_ERROR(r, {}, res, "/get_output_distribution.bin", error::wallet_generic_rpc_error, "/get_output_distribution.bin", get_rpc_status(m_trusted_daemon, res.status));
}
catch(...)
{
@@ -11872,7 +11872,7 @@ void wallet2::set_tx_key(const crypto::hash &txid, const crypto::secret_key &tx_
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
r = epee::net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client, rpc_timeout);
- THROW_ON_RPC_RESPONSE_ERROR_GENERIC(r, {}, res, "/gettransactions");
+ THROW_ON_RPC_RESPONSE_ERROR(r, {}, res, "/gettransactions", error::wallet_generic_rpc_error, "/gettransactions", get_rpc_status(m_trusted_daemon, res.status));
THROW_WALLET_EXCEPTION_IF(res.txs.size() != 1, error::wallet_internal_error,
"daemon returned wrong response for gettransactions, wrong txs count = " +
std::to_string(res.txs.size()) + ", expected 1");
@@ -11931,7 +11931,7 @@ std::string wallet2::get_spend_proof(const crypto::hash &txid, const std::string
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
r = epee::net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client, rpc_timeout);
- THROW_ON_RPC_RESPONSE_ERROR_GENERIC(r, {}, res, "gettransactions");
+ THROW_ON_RPC_RESPONSE_ERROR(r, {}, res, "gettransactions", error::wallet_generic_rpc_error, "gettransactions", get_rpc_status(m_trusted_daemon, res.status));
THROW_WALLET_EXCEPTION_IF(res.txs.size() != 1, error::wallet_internal_error,
"daemon returned wrong response for gettransactions, wrong txs count = " +
std::to_string(res.txs.size()) + ", expected 1");
@@ -12048,7 +12048,7 @@ bool wallet2::check_spend_proof(const crypto::hash &txid, const std::string &mes
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
r = epee::net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client, rpc_timeout);
- THROW_ON_RPC_RESPONSE_ERROR_GENERIC(r, {}, res, "gettransactions");
+ THROW_ON_RPC_RESPONSE_ERROR(r, {}, res, "gettransactions", error::wallet_generic_rpc_error, "gettransactions", get_rpc_status(m_trusted_daemon, res.status));
THROW_WALLET_EXCEPTION_IF(res.txs.size() != 1, error::wallet_internal_error,
"daemon returned wrong response for gettransactions, wrong txs count = " +
std::to_string(res.txs.size()) + ", expected 1");
@@ -15580,7 +15580,7 @@ std::vector<cryptonote::public_node> wallet2::get_public_nodes(bool white_only)
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
bool r = epee::net_utils::invoke_http_json("/get_public_nodes", req, res, *m_http_client, rpc_timeout);
- THROW_ON_RPC_RESPONSE_ERROR_GENERIC(r, {}, res, "/get_public_nodes");
+ THROW_ON_RPC_RESPONSE_ERROR(r, {}, res, "/get_public_nodes", error::wallet_generic_rpc_error, "/get_public_nodes", get_rpc_status(m_trusted_daemon, res.status));
}
std::vector<cryptonote::public_node> nodes;Why this scored 38/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.