read import blob crypto fields with memcpy in wallet2
What the developer wrote
Authored by alhudz
read import blob crypto fields with memcpy in wallet2
What changed, and why it matters
This commit changes how Monero's wallet reads cryptographic data from imported files. It replaces direct pointer casts with explicit memory copying (memcpy). This is a defensive coding fix that primarily addresses alignment and strict-aliasing issues, which can cause crashes or undefined behavior on some platforms. It is not a clear-cut remote exploit fix, but it removes a class of low-level memory-safety risks when loading wallet-related import data.
Treat as a hardening/defensive fix. Review whether additional input validation (size checks, format versioning, canonicalization) is needed for these import formats. No immediate emergency response is indicated unless the project is running on alignment-sensitive hardware or compiler optimizations are triggering strict-aliasing-related miscompilations.
Security signals we found
Replaces type-punning pointer casts with memcpy for cryptographic fields
Eliminates strict-aliasing violations and unaligned-load risks
Applies to import_key_images, import_outputs_from_str, and import_multisig
Defensive hardening of wallet import parsing code
Evidence from the diff
The patch modifies wallet2.cpp to read crypto::public_key, crypto::key_image, and crypto::signature fields from imported blobs using memcpy instead of reinterpret_cast pointer dereferences. The old pattern, e.g., (const crypto::public_key)&data[4], violates C/C++ strict-aliasing rules and may perform unaligned loads. The new pattern copies into properly aligned stack variables. This reduces undefined behavior and potential alignment faults, especially on architectures sensitive to unaligned access. The data being read is still attacker-controlled if the import file is malicious, but the change does not by itself prevent all parsing bugs.
Changed components
src/wallet/wallet2.cppimport_key_imagesimport_outputs_from_strimport_multisigInspect captured patch +14 / −9
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index d80dd57..f12466d 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -13227,8 +13227,9 @@ uint64_t wallet2::import_key_images(const std::string &filename, uint64_t &spent
const size_t headerlen = 4 + 2 * sizeof(crypto::public_key);
THROW_WALLET_EXCEPTION_IF(data.size() < headerlen, error::wallet_internal_error, std::string("Bad data size from file ") + filename);
const uint32_t offset = (uint8_t)data[0] | (((uint8_t)data[1]) << 8) | (((uint8_t)data[2]) << 16) | (((uint8_t)data[3]) << 24);
- const crypto::public_key &public_spend_key = *(const crypto::public_key*)&data[4];
- const crypto::public_key &public_view_key = *(const crypto::public_key*)&data[4 + sizeof(crypto::public_key)];
+ crypto::public_key public_spend_key, public_view_key;
+ memcpy(&public_spend_key, &data[4], sizeof(public_spend_key));
+ memcpy(&public_view_key, &data[4 + sizeof(crypto::public_key)], sizeof(public_view_key));
const cryptonote::account_public_address &keys = get_account().get_keys().m_account_address;
if (public_spend_key != keys.m_spend_public_key || public_view_key != keys.m_view_public_key)
{
@@ -13245,8 +13246,10 @@ uint64_t wallet2::import_key_images(const std::string &filename, uint64_t &spent
ski.reserve(nki);
for (size_t n = 0; n < nki; ++n)
{
- crypto::key_image key_image = *reinterpret_cast<const crypto::key_image*>(&data[headerlen + n * record_size]);
- crypto::signature signature = *reinterpret_cast<const crypto::signature*>(&data[headerlen + n * record_size + sizeof(crypto::key_image)]);
+ crypto::key_image key_image;
+ crypto::signature signature;
+ memcpy(&key_image, &data[headerlen + n * record_size], sizeof(key_image));
+ memcpy(&signature, &data[headerlen + n * record_size + sizeof(crypto::key_image)], sizeof(signature));
ski.push_back(std::make_pair(key_image, signature));
}
@@ -14313,8 +14316,9 @@ size_t wallet2::import_outputs_from_str(const std::string &outputs_st)
{
THROW_WALLET_EXCEPTION(error::wallet_internal_error, std::string("Bad data size for outputs"));
}
- const crypto::public_key &public_spend_key = *(const crypto::public_key*)&data[0];
- const crypto::public_key &public_view_key = *(const crypto::public_key*)&data[sizeof(crypto::public_key)];
+ crypto::public_key public_spend_key, public_view_key;
+ memcpy(&public_spend_key, &data[0], sizeof(public_spend_key));
+ memcpy(&public_view_key, &data[sizeof(crypto::public_key)], sizeof(public_view_key));
const cryptonote::account_public_address &keys = get_account().get_keys().m_account_address;
if (public_spend_key != keys.m_spend_public_key || public_view_key != keys.m_view_public_key)
{
@@ -14659,9 +14663,10 @@ size_t wallet2::import_multisig(std::vector<cryptonote::blobdata> blobs, bool re
const size_t headerlen = 3 * sizeof(crypto::public_key);
THROW_WALLET_EXCEPTION_IF(data.size() < headerlen, error::wallet_internal_error, "Bad data size");
- const crypto::public_key &public_spend_key = *(const crypto::public_key*)&data[0];
- const crypto::public_key &public_view_key = *(const crypto::public_key*)&data[sizeof(crypto::public_key)];
- const crypto::public_key &signer = *(const crypto::public_key*)&data[2*sizeof(crypto::public_key)];
+ crypto::public_key public_spend_key, public_view_key, signer;
+ memcpy(&public_spend_key, &data[0], sizeof(public_spend_key));
+ memcpy(&public_view_key, &data[sizeof(crypto::public_key)], sizeof(public_view_key));
+ memcpy(&signer, &data[2*sizeof(crypto::public_key)], sizeof(signer));
const cryptonote::account_public_address &keys = get_account().get_keys().m_account_address;
THROW_WALLET_EXCEPTION_IF(public_spend_key != keys.m_spend_public_key || public_view_key != keys.m_view_public_key,
error::wallet_internal_error, "Multisig info is for a different account");
Why this scored 38/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.