What the developer wrote
Authored by selsta
wallet2: remove deprecated txs_as_hex
also add missing underflow check
What changed, and why it matters
This Monero wallet code change removes an old way of receiving transaction data from the daemon (the network-facing full node) and switches to a newer, more structured format. It also adds a check to prevent a subtraction underflow when calculating how many blockchain confirmations a transaction has. The underflow could have caused incorrect confirmation counts, but the code throws an exception rather than silently misbehaving, so the practical security impact appears limited.
Treat as a routine hardening/refactoring patch. Review the `get_pruned_tx` helper for robustness, since it now handles all transaction parsing for these wallet proof/key functions. Verify that the new underflow guard is applied consistently wherever confirmation counts are computed from daemon height.
Security signals we found
Removal of deprecated hex transaction parsing path
Addition of unsigned integer underflow guard for confirmation count
Standardization on pruned transaction deserialization helper
Reduction in attack surface by eliminating `parse_and_validate_tx_from_blob` call sites in wallet RPC handling
Evidence from the diff
The commit removes all uses of the deprecated res.txs_as_hex fallback in wallet2.cpp’s /gettransactions handling and standardizes on res.txs with get_pruned_tx(). It also replaces local transaction parsing/validation with the helper and computes the prefix hash via get_transaction_prefix_hash(tx). A new guard bc_height > res.txs.front().block_height is added before computing confirmations = bc_height - block_height to prevent unsigned integer underflow. The change reduces code duplication and removes a legacy parsing path, but the diff alone does not demonstrate an exploitable vulnerability.
Changed components
src/wallet/wallet2.cppwallet2::get_tx_keywallet2::check_tx_key_helperwallet2::get_tx_proofwallet2::check_tx_proofInspect captured patch +13 / −52
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index a9a2629..3cd9ee5 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -11775,22 +11775,19 @@ bool wallet2::get_tx_key(const crypto::hash &txid, crypto::secret_key &tx_key, s
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
bool ok = epee::net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client);
- THROW_WALLET_EXCEPTION_IF(!ok || (res.txs.size() != 1 && res.txs_as_hex.size() != 1),
+ THROW_WALLET_EXCEPTION_IF(!ok || res.txs.size() != 1,
error::wallet_internal_error, "Failed to get transaction from daemon");
}
cryptonote::transaction tx;
crypto::hash tx_hash{};
cryptonote::blobdata tx_data;
- crypto::hash tx_prefix_hash{};
- bool ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data);
+ bool ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx, tx_hash, tx_prefix_hash),
- error::wallet_internal_error, "Failed to validate transaction from daemon");
THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error,
"Failed to get the right transaction from daemon");
- tx_key_data.tx_prefix_hash = std::string(tx_prefix_hash.data, 32);
+ tx_key_data.tx_prefix_hash = std::string(get_transaction_prefix_hash(tx).data, 32);
}
std::vector<crypto::secret_key> tx_keys;
@@ -12157,26 +12154,14 @@ void wallet2::check_tx_key_helper(const crypto::hash &txid, const crypto::key_de
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
ok = epee::net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client);
- THROW_WALLET_EXCEPTION_IF(!ok || (res.txs.size() != 1 && res.txs_as_hex.size() != 1),
+ THROW_WALLET_EXCEPTION_IF(!ok || res.txs.size() != 1,
error::wallet_internal_error, "Failed to get transaction from daemon");
}
cryptonote::transaction tx;
crypto::hash tx_hash;
- if (res.txs.size() == 1)
- {
- ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- }
- else
- {
- cryptonote::blobdata tx_data;
- ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx),
- error::wallet_internal_error, "Failed to validate transaction from daemon");
- tx_hash = cryptonote::get_transaction_hash(tx);
- }
+ ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
+ THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error,
"Failed to get the right transaction from daemon");
@@ -12252,26 +12237,14 @@ std::string wallet2::get_tx_proof(const crypto::hash &txid, const cryptonote::ac
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
ok = net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client);
- THROW_WALLET_EXCEPTION_IF(!ok || (res.txs.size() != 1 && res.txs_as_hex.size() != 1),
+ THROW_WALLET_EXCEPTION_IF(!ok || res.txs.size() != 1,
error::wallet_internal_error, "Failed to get transaction from daemon");
}
cryptonote::transaction tx;
crypto::hash tx_hash;
- if (res.txs.size() == 1)
- {
- ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- }
- else
- {
- cryptonote::blobdata tx_data;
- ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx),
- error::wallet_internal_error, "Failed to validate transaction from daemon");
- tx_hash = cryptonote::get_transaction_hash(tx);
- }
+ ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
+ THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error, "Failed to get the right transaction from daemon");
@@ -12410,26 +12383,14 @@ bool wallet2::check_tx_proof(const crypto::hash &txid, const cryptonote::account
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
ok = net_utils::invoke_http_json("/gettransactions", req, res, *m_http_client);
- THROW_WALLET_EXCEPTION_IF(!ok || (res.txs.size() != 1 && res.txs_as_hex.size() != 1),
+ THROW_WALLET_EXCEPTION_IF(!ok || res.txs.size() != 1,
error::wallet_internal_error, "Failed to get transaction from daemon");
}
cryptonote::transaction tx;
crypto::hash tx_hash;
- if (res.txs.size() == 1)
- {
- ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- }
- else
- {
- cryptonote::blobdata tx_data;
- ok = string_tools::parse_hexstr_to_binbuff(res.txs_as_hex.front(), tx_data);
- THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
- THROW_WALLET_EXCEPTION_IF(!cryptonote::parse_and_validate_tx_from_blob(tx_data, tx),
- error::wallet_internal_error, "Failed to validate transaction from daemon");
- tx_hash = cryptonote::get_transaction_hash(tx);
- }
+ ok = get_pruned_tx(res.txs.front(), tx, tx_hash);
+ THROW_WALLET_EXCEPTION_IF(!ok, error::wallet_internal_error, "Failed to parse transaction from daemon");
THROW_WALLET_EXCEPTION_IF(tx_hash != txid, error::wallet_internal_error, "Failed to get the right transaction from daemon");
@@ -12442,7 +12403,7 @@ bool wallet2::check_tx_proof(const crypto::hash &txid, const cryptonote::account
{
std::string err;
uint64_t bc_height = get_daemon_blockchain_height(err);
- if (err.empty())
+ if (err.empty() && bc_height > res.txs.front().block_height)
confirmations = bc_height - res.txs.front().block_height;
}
Why this scored 42/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.