From 44412d3574fcd7bdb6d0fcfb6c4ffc61c2d338d9 Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Tue, 21 Apr 2026 01:57:21 -0500 Subject: cryptonote_core: add change address sanity check Prevents silly mistakes where wrong change address is passed Release versions uses boost::optional instead of std::optional --- src/cryptonote_core/cryptonote_tx_utils.cpp | 44 +++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'src/cryptonote_core/cryptonote_tx_utils.cpp') diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp index 8f044154b..c350e24c4 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.cpp +++ b/src/cryptonote_core/cryptonote_tx_utils.cpp @@ -31,6 +31,7 @@ #include #include #include "include_base_utils.h" +#include "misc_log_ex.h" #include "string_tools.h" using namespace epee; @@ -46,6 +47,40 @@ using namespace epee; using namespace crypto; + +namespace +{ +//--------------------------------------------------------------- +/** + * @brief check if can re-derive change address from device / keys + * @param change_addr address to attempt to re-derive + * @param subaddresses subaddress map + * @param keys account keys of sender + * @return subaddress index of `change_addr` if in the subaddress map and re-derives from device, otherwise nullopt + */ +boost::optional sanity_check_change_address( + const cryptonote::account_public_address& change_addr, + const std::unordered_map& subaddresses, + const cryptonote::account_keys &keys +) +{ + // guess/find subaddress index of `change_addr`, works for main addresses if `subaddresses` is empty + cryptonote::subaddress_index subaddr_index{}; // (0, 0) by default + const auto subaddr_it = subaddresses.find(change_addr.m_spend_public_key); + if (subaddr_it != subaddresses.cend()) + subaddr_index = subaddr_it->second; + + // if device does not return same address given index, then fail + hw::device &hwdev = keys.get_device(); + const auto recomputed_addr = hwdev.get_subaddress(keys, subaddr_index); + if (change_addr != recomputed_addr) + return boost::none; + + return boost::optional(subaddr_index); +} +//--------------------------------------------------------------- +} //anonymous namespace + namespace cryptonote { //--------------------------------------------------------------- @@ -213,6 +248,10 @@ namespace cryptonote return false; } + boost::optional recognized_change_index; + if (change_addr) + recognized_change_index = sanity_check_change_address(*change_addr, subaddresses, sender_account_keys); + std::vector amount_keys; tx.set_null(); amount_keys.clear(); @@ -406,6 +445,11 @@ namespace cryptonote for(const tx_destination_entry& dst_entr: destinations) { CHECK_AND_ASSERT_MES(dst_entr.amount > 0 || tx.version > 1, false, "Destination with wrong amount: " << dst_entr.amount); + const bool matches_change_addr = change_addr && dst_entr.addr == *change_addr; + const bool is_bad_change_dst = matches_change_addr && dst_entr.amount > 0 && !recognized_change_index; + CHECK_AND_ASSERT_MES(!is_bad_change_dst, false, + "Non-zero amount change address is not recognized as belonging to the sender account"); + crypto::public_key out_eph_public_key; crypto::view_tag view_tag; -- cgit v1.2.3