wallet2: use decodeRct for reserve proof amount validation
What changed, and why it matters
This commit changes how the Monero wallet validates hidden transaction amounts in two places: when checking a transaction key and when verifying a reserve proof. It replaces custom code that manually decoded encrypted amounts with a shared helper function called decodeRct. The stated goal is consistency and using the standard decoding path. The change removes several manual safety checks (for example, checks that the encrypted mask and amount values are valid curve scalars) and no longer verifies that the decoded amount matches the transaction's public commitment. That could, in theory, allow a maliciously crafted proof or transaction to make the wallet accept an incorrect amount, though the practical exploit path is unclear without more context.
Review the implementation of rct::decodeRct to confirm it performs equivalent scalar validation and commitment verification for the relevant RCT types. If decodeRct does not enforce those checks for all transaction types, consider restoring them or ensuring the helper is safe for these call sites. Test reserve proof verification against crafted proofs that supply invalid ECDH tuples or mismatching commitments. If this change fixes a known security bug, request a CVE and vendor advisory.
Security signals we found
Removal of manual sc_check validation on ECDH mask and amount scalars
Removal of on-chain Pedersen commitment equality check in amount decoding path
Delegation to decodeRct, a shared helper, which may or may not preserve the same validation
Change affects reserve proof verification, which is used to prove ownership of funds without revealing the wallet's view key
Change also affects check_tx_key_helper, used to verify incoming transaction amounts
Evidence from the diff
The patch refactors wallet2::check_tx_key_helper and wallet2::check_reserve_proof to call rct::decodeRct instead of inline ECDH decoding and commitment verification. In check_tx_key_helper, the old code computed a scalar from the derivation, decoded the ecdhInfo, ran sc_check on both mask and amount, recomputed the output commitment (addKeys2), compared it to the on-chain commitment (equalKeys), and only then accepted the decoded amount. The new code delegates all of that to decodeRct, which is the normal wallet path for amount decoding. In check_reserve_proof, the old code similarly decoded ecdhInfo and directly converted the amount scalar to an integer without commitment verification. The new code also uses decodeRct. The commit message says this is for reserve proof amount validation and consistency. The diff itself does not show decodeRct’s implementation, so we cannot confirm whether it preserves the dropped checks. No CVE, advisory, or vendor security statement is present in the supplied materials.
Changed components
src/wallet/wallet2.cppwallet2::check_tx_key_helperwallet2::check_reserve_proofringct amount decoding (decodeRct)Inspect captured patch +4 / −18
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index b073357..fc17561 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -12120,19 +12120,8 @@ void wallet2::check_tx_key_helper(const cryptonote::transaction &tx, const crypt
}
else
{
- crypto::secret_key scalar1;
- crypto::derivation_to_scalar(found_derivation, n, scalar1);
- rct::ecdhTuple ecdh_info = tx.rct_signatures.ecdhInfo[n];
- rct::ecdhDecode(ecdh_info, rct::sk2rct(scalar1), tx.rct_signatures.type == rct::RCTTypeBulletproof2 || tx.rct_signatures.type == rct::RCTTypeCLSAG || tx.rct_signatures.type == rct::RCTTypeBulletproofPlus);
- const rct::key C = tx.rct_signatures.outPk[n].mask;
- rct::key Ctmp;
- THROW_WALLET_EXCEPTION_IF(sc_check(ecdh_info.mask.bytes) != 0, error::wallet_internal_error, "Bad ECDH input mask");
- THROW_WALLET_EXCEPTION_IF(sc_check(ecdh_info.amount.bytes) != 0, error::wallet_internal_error, "Bad ECDH input amount");
- rct::addKeys2(Ctmp, ecdh_info.mask, ecdh_info.amount, rct::H);
- if (rct::equalKeys(C, Ctmp))
- amount = rct::h2d(ecdh_info.amount);
- else
- amount = 0;
+ rct::key mask;
+ amount = decodeRct(tx.rct_signatures, found_derivation, n, mask, hw::get_device("default"));
}
received += amount;
}
@@ -12753,11 +12742,8 @@ bool wallet2::check_reserve_proof(const cryptonote::account_public_address &addr
if (amount == 0)
{
// decode rct
- crypto::secret_key shared_secret;
- crypto::derivation_to_scalar(derivation, proof.index_in_tx, shared_secret);
- rct::ecdhTuple ecdh_info = tx.rct_signatures.ecdhInfo[proof.index_in_tx];
- rct::ecdhDecode(ecdh_info, rct::sk2rct(shared_secret), tx.rct_signatures.type == rct::RCTTypeBulletproof2 || tx.rct_signatures.type == rct::RCTTypeCLSAG || tx.rct_signatures.type == rct::RCTTypeBulletproofPlus);
- amount = rct::h2d(ecdh_info.amount);
+ rct::key mask_;
+ amount = decodeRct(tx.rct_signatures, derivation, proof.index_in_tx, mask_, hw::get_device("default"));
}
total += amount;
if (kispent_res.spent_status[i])
Why this scored 41/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.