wallet: derive encrypted payment ID dummy/real status from tx.extra, not cd.dests
What the developer wrote
Authored by waris )
wallet: derive encrypted payment ID dummy/real status from tx.extra, not cd.dests
What changed, and why it matters
This Monero wallet patch changes how the wallet decides whether an encrypted payment ID is real or a dummy placeholder. Previously, the wallet relied on destination address data (cd.dests), which could be manipulated by a malicious signer or loaded transaction to make a real payment ID look fake, or a fake one look real. The patch now derives that status directly from the transaction's extra field, where the payment ID is actually stored, and adds a consistency check. This is a security fix for a potential information leak or user deception during multi-step transaction signing.
Treat as a security fix. Review related code paths for other instances where destination metadata rather than tx.extra is used for payment ID decisions, and verify the consistency check cannot be bypassed. No explicit CVE or advisory is present in the provided materials, so monitor Monero Project channels for disclosure.
Security signals we found
Trust boundary crossed: loaded unsigned transaction / destination metadata used for security-relevant decision
Inconsistent data sources for payment ID status (tx.extra vs cd.dests)
Added runtime consistency assertion (CHECK_AND_ASSERT_MES) between derived dummy status and actual encrypted payment ID value
Removed reliance on entry.original address string comparison for integrated address display
Potential for user deception: real encrypted payment ID could be hidden or dummy could be presented as real
Evidence from the diff
The commit modifies three wallet components (simplewallet, unsigned transaction API, wallet RPC server) so that encrypted payment ID dummy/real status is determined from tx.extra rather than from destination metadata. It adds a CHECK_AND_ASSERT_MES consistency check in simplewallet ensuring is_dummy matches (payment_id8 == crypto::null_hash8). It also removes comparisons to entry.original when deciding whether to display an integrated address, instead relying solely on has_encrypted_payment_id and !entry.is_subaddress. The unsigned transaction API now distinguishes dummy encrypted payment IDs from real ones. This closes a trust boundary issue where a co-signer or loaded unsigned tx provider could misrepresent payment ID status.
Changed components
src/simplewallet/simplewallet.cppsrc/wallet/api/unsigned_transaction.cppsrc/wallet/wallet_rpc_server.cppInspect captured patch +13 / −4
diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp
index 5985f63..f16434c 100644
--- a/src/simplewallet/simplewallet.cpp
+++ b/src/simplewallet/simplewallet.cpp
@@ -7477,6 +7477,8 @@ bool simple_wallet::accept_loaded_tx(const std::function<size_t()> get_num_txes,
if (e.is_integrated)
is_dummy = false;
+ CHECK_AND_ASSERT_MES(is_dummy == (payment_id8 == crypto::null_hash8), false, "Bad loaded tx: mismatched payment ID info");
+
if (is_dummy)
{
payment_id_string += std::string("dummy encrypted payment ID");
@@ -7508,7 +7510,7 @@ bool simple_wallet::accept_loaded_tx(const std::function<size_t()> get_num_txes,
{
const tx_destination_entry &entry = cd.splitted_dsts[d];
std::string address, standard_address = get_account_address_as_str(m_wallet->nettype(), entry.is_subaddress, entry.addr);
- if (has_encrypted_payment_id && !entry.is_subaddress && standard_address != entry.original)
+ if (has_encrypted_payment_id && !entry.is_subaddress)
{
address = get_account_integrated_address_as_str(m_wallet->nettype(), entry.addr, payment_id8);
address += std::string(" (" + standard_address + " with encrypted payment id " + epee::string_tools::pod_to_hex(payment_id8) + ")");
diff --git a/src/wallet/api/unsigned_transaction.cpp b/src/wallet/api/unsigned_transaction.cpp
index c549539..a8d8bbf 100644
--- a/src/wallet/api/unsigned_transaction.cpp
+++ b/src/wallet/api/unsigned_transaction.cpp
@@ -122,8 +122,15 @@ bool UnsignedTransactionImpl::checkLoadedTx(const std::function<size_t()> get_nu
{
if (!payment_id_string.empty())
payment_id_string += ", ";
- payment_id_string = std::string("encrypted payment ID ") + epee::string_tools::pod_to_hex(payment_id8);
- has_encrypted_payment_id = true;
+ if (payment_id8 == crypto::null_hash8)
+ {
+ payment_id_string += std::string("dummy encrypted payment ID");
+ }
+ else
+ {
+ payment_id_string += std::string("encrypted payment ID ") + epee::string_tools::pod_to_hex(payment_id8);
+ has_encrypted_payment_id = true;
+ }
}
else if (cryptonote::get_payment_id_from_tx_extra_nonce(extra_nonce.nonce, payment_id))
{
diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp
index 2dae20f..5f42c0b 100644
--- a/src/wallet/wallet_rpc_server.cpp
+++ b/src/wallet/wallet_rpc_server.cpp
@@ -1580,7 +1580,7 @@ namespace tools
{
const cryptonote::tx_destination_entry &entry = cd.splitted_dsts[d];
std::string address = cryptonote::get_account_address_as_str(m_wallet->nettype(), entry.is_subaddress, entry.addr);
- if (has_encrypted_payment_id && !entry.is_subaddress && address != entry.original)
+ if (has_encrypted_payment_id && !entry.is_subaddress)
address = cryptonote::get_account_integrated_address_as_str(m_wallet->nettype(), entry.addr, payment_id8);
auto i = tx_dests.find(entry.addr);
if (i == tx_dests.end())
Why this scored 59/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.