What the developer wrote
Authored by tobtoht
Merge pull request #11319
fe371dd wallet2: filter unchecked RPC error statuses (selsta)
dab064f wallet2: validate public node addresses before display (selsta)
5834007 common: sanitize message writer console output (selsta)
ACKs: SNeedlewoods, thomasbuilds, jpk68
What changed, and why it matters
This Monero wallet update fixes three separate issues: it prevents weird or malicious text from log/console messages from confusing the terminal (terminal escape-sequence injection), it stops the wallet from trusting and displaying invalid public node addresses from the network, and it improves error handling when the wallet talks to a daemon so that untrusted RPC responses are not silently accepted. None of these directly steal funds, but they reduce ways an attacker could trick a user or hide problems.
Treat as a routine security-hardening patch. Users running monero-wallet-cli or wallet2-based wallets should upgrade. Operators should verify that public node lists now filter invalid entries and that daemon error statuses are reported correctly. No immediate emergency response is indicated, but the patch should be included in the next release.
Security signals we found
Sanitization of console/log output to remove control characters and invalid UTF-8
Validation and normalization of externally supplied public node addresses before display/use
Replacement of generic RPC error handling with explicit status checks on untrusted daemon responses
IPv6 address bracketing in UI output
Evidence from the diff
The merge contains three logical fixes. (1) easylogging++ and scoped_message_writer now sanitize UTF-8 and non-printable characters before writing to console/log files, closing a terminal escape-injection / log-injection path. (2) wallet2::get_public_nodes validates host/port pairs using net::get_network_address and IPv6 parsing, discarding nodes with rpc_port==0 or unparseable hosts, and normalizes host strings before display. (3) Several wallet2 RPC call sites switch from THROW_ON_RPC_RESPONSE_ERROR_GENERIC to THROW_ON_RPC_RESPONSE_ERROR with explicit error codes and get_rpc_status(m_trusted_daemon, res.status), so unchecked daemon error statuses are surfaced rather than ignored. The public-node change also brackets IPv6 addresses in the simplewallet display.
Changed components
external/easylogging++/easylogging++.ccexternal/easylogging++/easylogging++.hsrc/common/scoped_message_writer.hsrc/simplewallet/simplewallet.cppsrc/wallet/wallet2.cppInspect captured patch +36 / −14
### external/easylogging++/easylogging++.cc
@@ -16,6 +16,7 @@
#define EASYLOGGING_CC
#include "easylogging++.h"
+#include <cstdint>
#include <atomic>
#include <unistd.h>
@@ -2502,11 +2503,11 @@ void DefaultLogDispatchCallback::handle(const LogDispatchData* data) {
template<typename Transform>
-static inline void utf8canonical(std::string &s, Transform t = [](wint_t c)->wint_t { return c; })
+static inline void utf8canonical(std::string &s, Transform t = [](std::uint32_t c)->std::uint32_t { return c; })
{
size_t avail = s.size();
const char *ptr = s.data();
- wint_t cp = 0;
+ std::uint32_t cp = 0;
int rbytes = 1, bytes = -1;
char wbuf[8], *wptr;
size_t w_offset = 0;
@@ -2585,7 +2586,7 @@ static inline void utf8canonical(std::string &s, Transform t = [](wint_t c)->win
void sanitize(std::string &s)
{
- utf8canonical(s, [](wint_t c)->wint_t {
+ utf8canonical(s, [](std::uint32_t c)->std::uint32_t {
if (c == 9 || c == 10 || c == 13)
return c;
if (c < 0x20)
### external/easylogging++/easylogging++.h
@@ -2792,6 +2792,7 @@ class Storage : base::NoCopy, public base::threading::ThreadSafe {
}
};
extern ELPP_EXPORT base::type::StoragePointer elStorage;
+ELPP_EXPORT void sanitize(std::string& s);
#define ELPP el::base::Storage::getELPP()
class DefaultLogDispatchCallback : public LogDispatchCallback {
protected:
### src/common/scoped_message_writer.h
@@ -95,17 +95,20 @@ class scoped_message_writer
{
m_flush = false;
- MCLOG_FILE(m_log_level, "msgwriter", m_oss.str());
+ std::string message = m_oss.str();
+ MCLOG_FILE(m_log_level, "msgwriter", message);
+ try { el::base::sanitize(message); }
+ catch (const std::exception&) { message = "<Invalid UTF-8 in message>"; }
PAUSE_READLINE();
if (epee::console_color_default == m_color)
{
- std::cout << m_oss.str();
+ std::cout << message;
}
else
{
set_console_color(m_color, m_bright);
- std::cout << m_oss.str();
+ std::cout << message;
epee::reset_console_color();
}
std::cout << std::endl;
### src/simplewallet/simplewallet.cpp
@@ -2148,8 +2148,9 @@ bool simple_wallet::public_nodes(const std::vector<std::string> &args)
message_writer() << boost::format("%32s %16s") % tr("address") % tr("last_seen");
for (const auto &node: nodes)
{
- const std::string last_seen = node.last_seen == 0 ? tr("never") : tools::get_human_readable_timespan(std::chrono::seconds(now - node.last_seen));
- std::string host = node.host + ":" + std::to_string(node.rpc_port);
+ const std::string last_seen = node.last_seen == 0 ? tr("never") : tools::get_human_readable_timespan(std::chrono::seconds(now - std::min(now, node.last_seen)));
+ const std::string host = (node.host.find(':') == std::string::npos ? node.host : "[" + node.host + "]")
+ + ":" + std::to_string(node.rpc_port);
message_writer() << boost::format("%32s %16s") % host % last_seen;
}
}
### src/wallet/wallet2.cpp
@@ -4421,7 +4421,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(...)
{
@@ -11988,7 +11988,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");
@@ -12047,7 +12047,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");
@@ -12164,7 +12164,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");
@@ -15743,14 +15743,30 @@ 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;
nodes = res.white;
nodes.reserve(nodes.size() + res.gray.size());
std::copy(res.gray.begin(), res.gray.end(), std::back_inserter(nodes));
- return nodes;
+ std::vector<cryptonote::public_node> valid_nodes;
+ valid_nodes.reserve(nodes.size());
+ for (auto &node: nodes)
+ {
+ if (node.rpc_port == 0)
+ continue;
+ auto address = net::get_network_address(node.host, node.rpc_port);
+ boost::system::error_code ec;
+ const auto ipv6 = boost::asio::ip::make_address_v6(node.host, ec);
+ if (!ec)
+ address = epee::net_utils::network_address{epee::net_utils::ipv6_network_address{ipv6, node.rpc_port}};
+ if (!address || (node.host != address->host_str() && node.host != address->str()))
+ continue;
+ node.host = address->host_str();
+ valid_nodes.push_back(std::move(node));
+ }
+ return valid_nodes;
}
//----------------------------------------------------------------------------------------------------
std::pair<size_t, uint64_t> wallet2::estimate_tx_size_and_weight(bool use_rct, int n_inputs, int ring_size, int n_outputs, size_t extra_size)Why this scored 48/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.