Public commit record
45/100 · Thin
What the developer wrote
Authored by Ap4sh
wallet: copy parsed crypto fields
✓ Descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit changes how Monero's wallet code copies sensitive cryptographic data (secret keys, transaction hashes, payment IDs) from one memory location to another. Previously it used direct pointer casting, which can violate strict type alignment rules and may cause undefined behavior on some platforms. The patch replaces those casts with explicit memory copies (memcpy). This is a defensive hardening change: it reduces the risk of crashes or misaligned memory reads, but the commit itself does not claim to fix an active exploit or known vulnerability.
Recommended action
Treat as a defensive hardening patch. Review whether the old reinterpret_cast pattern exists elsewhere in the codebase and apply the same memcpy pattern consistently. No urgent incident response is indicated by the supplied materials, but users running builds on strict-alignment architectures may benefit from the fix.
Security signals we found
01
Replacement of reinterpret_cast with memcpy for cryptographic POD types
02
Addition of unit tests for payment ID parsing failure handling
03
No explicit security claim or CVE reference in commit message
04
No vendor advisory or researcher attribution in supplied materials
Evidence from the diff
The diff replaces reinterpret_cast-based loads of crypto::hash, crypto::hash8, and crypto::secret_key from std::string/blobdata buffers with memcpy. The affected paths are wallet API key validation, wallet recovery from keys, transaction note lookup by txid, and payment ID parsing. A unit test is added to verify that parse_long_payment_id and parse_short_payment_id reject malformed inputs and do not modify the output on failure. The change is consistent with C++ strict-aliasing/alignment safety best practices, but the commit message is purely descriptive and does not disclose a security bug.
Changed components
src/wallet/api/transaction_history.cppsrc/wallet/api/wallet.cppsrc/wallet/wallet2.cpptests/unit_tests/uri.cppInspect captured patch +56 / −11
diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp
index 4dc869d..0108aa6 100644
--- a/src/wallet/api/transaction_history.cpp
+++ b/src/wallet/api/transaction_history.cpp
@@ -36,7 +36,7 @@
#include "crypto/hash.h"
#include "wallet/wallet2.h"
-
+#include <cstring>
#include <string>
#include <list>
@@ -97,7 +97,8 @@ void TransactionHistoryImpl::setTxNote(const std::string &txid, const std::strin
cryptonote::blobdata txid_data;
if(!epee::string_tools::parse_hexstr_to_binbuff(txid, txid_data) || txid_data.size() != sizeof(crypto::hash))
return;
- const crypto::hash htxid = *reinterpret_cast<const crypto::hash*>(txid_data.data());
+ crypto::hash htxid;
+ memcpy(&htxid, txid_data.data(), sizeof(htxid));
m_wallet->m_wallet->set_tx_note(htxid, note);
refresh();
diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp
index bad6f5a..7f03f42 100644
--- a/src/wallet/api/wallet.cpp
+++ b/src/wallet/api/wallet.cpp
@@ -43,6 +43,7 @@
#include "mnemonics/electrum-words.h"
#include "mnemonics/english.h"
#include <boost/format.hpp>
+#include <cstring>
#include <sstream>
#include <unordered_map>
@@ -363,7 +364,8 @@ bool Wallet::keyValid(const std::string &secret_key_string, const std::string &a
error = tr("Failed to parse key");
return false;
}
- crypto::secret_key key = *reinterpret_cast<const crypto::secret_key*>(key_data.data());
+ crypto::secret_key key;
+ memcpy(&unwrap(unwrap(key)), key_data.data(), sizeof(key));
// check the key match the given address
crypto::public_key pkey;
@@ -613,7 +615,7 @@ bool WalletImpl::recoverFromKeysWithPassword(const std::string &path,
return false;
}
has_spendkey = true;
- spendkey = *reinterpret_cast<const crypto::secret_key*>(spendkey_data.data());
+ memcpy(&unwrap(unwrap(spendkey)), spendkey_data.data(), sizeof(spendkey));
}
// parse view secret key
@@ -635,7 +637,7 @@ bool WalletImpl::recoverFromKeysWithPassword(const std::string &path,
setStatusError(tr("failed to parse secret view key"));
return false;
}
- viewkey = *reinterpret_cast<const crypto::secret_key*>(viewkey_data.data());
+ memcpy(&unwrap(unwrap(viewkey)), viewkey_data.data(), sizeof(viewkey));
}
// check the spend and view keys match the given address
crypto::public_key pkey;
@@ -1963,7 +1965,8 @@ bool WalletImpl::setUserNote(const std::string &txid, const std::string ¬e)
cryptonote::blobdata txid_data;
if(!epee::string_tools::parse_hexstr_to_binbuff(txid, txid_data) || txid_data.size() != sizeof(crypto::hash))
return false;
- const crypto::hash htxid = *reinterpret_cast<const crypto::hash*>(txid_data.data());
+ crypto::hash htxid;
+ memcpy(&htxid, txid_data.data(), sizeof(htxid));
m_wallet->set_tx_note(htxid, note);
return true;
@@ -1976,7 +1979,8 @@ std::string WalletImpl::getUserNote(const std::string &txid) const
cryptonote::blobdata txid_data;
if(!epee::string_tools::parse_hexstr_to_binbuff(txid, txid_data) || txid_data.size() != sizeof(crypto::hash))
return "";
- const crypto::hash htxid = *reinterpret_cast<const crypto::hash*>(txid_data.data());
+ crypto::hash htxid;
+ memcpy(&htxid, txid_data.data(), sizeof(htxid));
return m_wallet->get_tx_note(htxid);
}
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index c128eda..9bbf4cc 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -582,7 +582,7 @@ std::pair<std::unique_ptr<tools::wallet2>, tools::password_container> generate_f
{
THROW_WALLET_EXCEPTION(tools::error::wallet_internal_error, tools::wallet2::tr("failed to parse view key secret key"));
}
- viewkey = *reinterpret_cast<const crypto::secret_key*>(viewkey_data.data());
+ memcpy(&unwrap(unwrap(viewkey)), viewkey_data.data(), sizeof(viewkey));
crypto::public_key pkey;
if (viewkey == crypto::null_skey)
THROW_WALLET_EXCEPTION(tools::error::wallet_internal_error, tools::wallet2::tr("view secret key may not be all zeroes"));
@@ -600,7 +600,7 @@ std::pair<std::unique_ptr<tools::wallet2>, tools::password_container> generate_f
{
THROW_WALLET_EXCEPTION(tools::error::wallet_internal_error, tools::wallet2::tr("failed to parse spend key secret key"));
}
- spendkey = *reinterpret_cast<const crypto::secret_key*>(spendkey_data.data());
+ memcpy(&unwrap(unwrap(spendkey)), spendkey_data.data(), sizeof(spendkey));
crypto::public_key pkey;
if (spendkey == crypto::null_skey)
THROW_WALLET_EXCEPTION(tools::error::wallet_internal_error, tools::wallet2::tr("spend secret key may not be all zeroes"));
@@ -6298,7 +6298,7 @@ bool wallet2::parse_long_payment_id(const std::string& payment_id_str, crypto::h
if(sizeof(crypto::hash) != payment_id_data.size())
return false;
- payment_id = *reinterpret_cast<const crypto::hash*>(payment_id_data.data());
+ memcpy(&payment_id, payment_id_data.data(), sizeof(payment_id));
return true;
}
//----------------------------------------------------------------------------------------------------
@@ -6311,7 +6311,7 @@ bool wallet2::parse_short_payment_id(const std::string& payment_id_str, crypto::
if(sizeof(crypto::hash8) != payment_id_data.size())
return false;
- payment_id = *reinterpret_cast<const crypto::hash8*>(payment_id_data.data());
+ memcpy(&payment_id, payment_id_data.data(), sizeof(payment_id));
return true;
}
//----------------------------------------------------------------------------------------------------
diff --git a/tests/unit_tests/uri.cpp b/tests/unit_tests/uri.cpp
index f1c2b69..3560ec6 100644
--- a/tests/unit_tests/uri.cpp
+++ b/tests/unit_tests/uri.cpp
@@ -27,6 +27,7 @@
// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "gtest/gtest.h"
+#include "string_tools.h"
#include "wallet/wallet2.h"
#define TEST_ADDRESS "9tTLtauaEKSj7xoVXytVH32R1pLZBk4VV4mZFGEh4wkXhDWqw1soPyf3fGixf1kni31VznEZkWNEza9d5TvjWwq5PaohYHC"
@@ -154,6 +155,45 @@ TEST(uri, long_payment_id)
ASSERT_EQ(payment_id, "1234567890123456789012345678901234567890123456789012345678901234");
}
+TEST(wallet2, parse_long_payment_id)
+{
+ const std::string payment_id_hex = "00112233445566778899aabbccddeeffffeeddccbbaa99887766554433221100";
+ crypto::hash payment_id = crypto::null_hash;
+
+ ASSERT_TRUE(tools::wallet2::parse_long_payment_id(payment_id_hex, payment_id));
+ EXPECT_EQ(payment_id_hex, epee::string_tools::pod_to_hex(payment_id));
+
+ const crypto::hash unchanged = payment_id;
+ EXPECT_FALSE(tools::wallet2::parse_long_payment_id(payment_id_hex.substr(2), payment_id));
+ EXPECT_EQ(unchanged, payment_id);
+ EXPECT_FALSE(tools::wallet2::parse_long_payment_id(std::string(64, 'z'), payment_id));
+ EXPECT_EQ(unchanged, payment_id);
+}
+
+TEST(wallet2, parse_short_payment_id)
+{
+ const std::string payment_id_hex = "0011223344556677";
+ crypto::hash8 payment_id = crypto::null_hash8;
+
+ ASSERT_TRUE(tools::wallet2::parse_short_payment_id(payment_id_hex, payment_id));
+ EXPECT_EQ(payment_id_hex, epee::string_tools::pod_to_hex(payment_id));
+
+ const crypto::hash8 unchanged = payment_id;
+ EXPECT_FALSE(tools::wallet2::parse_short_payment_id(payment_id_hex.substr(2), payment_id));
+ EXPECT_EQ(unchanged, payment_id);
+ EXPECT_FALSE(tools::wallet2::parse_short_payment_id(std::string(16, 'z'), payment_id));
+ EXPECT_EQ(unchanged, payment_id);
+}
+
+TEST(wallet2, parse_payment_id_pads_short_ids)
+{
+ const std::string payment_id_hex = "0011223344556677";
+ crypto::hash payment_id = crypto::null_hash;
+
+ ASSERT_TRUE(tools::wallet2::parse_payment_id(payment_id_hex, payment_id));
+ EXPECT_EQ(payment_id_hex + std::string(48, '0'), epee::string_tools::pod_to_hex(payment_id));
+}
+
TEST(uri, payment_id_with_integrated_address)
{
PARSE_URI("monero:" TEST_INTEGRATED_ADDRESS"?tx_payment_id=1234567890123456", false);
Risk score
Our methodology →Why this scored 46/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.