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

1 files changed, 44 insertions, 0 deletions

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 <unordered_set>
 #include <random>
 #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<cryptonote::subaddress_index> sanity_check_change_address(
+  const cryptonote::account_public_address& change_addr,
+  const std::unordered_map<crypto::public_key, cryptonote::subaddress_index>& 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<cryptonote::subaddress_index>(subaddr_index);
+}
+//---------------------------------------------------------------
+} //anonymous namespace
+
 namespace cryptonote
 {
   //---------------------------------------------------------------
@@ -213,6 +248,10 @@ namespace cryptonote
       return false;
     }
 
+    boost::optional<cryptonote::subaddress_index> recognized_change_index;
+    if (change_addr)
+      recognized_change_index = sanity_check_change_address(*change_addr, subaddresses, sender_account_keys);
+
     std::vector<rct::key> 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;