wallet2: store multisig nonce erasure before returning signed txset
What changed, and why it matters
This patch changes how Monero's wallet handles multisig transactions. In a multisig wallet, several people must cooperate to sign a transaction. The patch makes sure that sensitive one-time secret values (called 'nonces') are wiped from the wallet's memory and saved to disk *before* the partially-signed transaction file is handed back to the user. Previously, the wallet could return the signed transaction file while still keeping those secret values in memory. If the wallet later crashed or was used again without saving, those secret values might be reused or leaked, which could weaken the security of the multisig scheme and, in the worst case, allow an attacker to recover private key material or forge signatures.
Treat this as a security fix and include it in the next release. Users who create or sign multisig transactions with affected versions should upgrade promptly. Wallet developers building on wallet2 should ensure they do not expose signed/partial multisig txsets before erasing and storing the corresponding m_multisig_k values. No immediate on-chain action is required.
Security signals we found
Multisig nonce (k-value) erasure moved to occur before txset is exposed to caller
New helper clear_multisig_k_and_store() ensures wallet cache is persisted after erasure
sign_multisig_tx now operates on a local copy and only commits output after secure cleanup
m_tx_keys/m_additional_tx_keys updates deferred until after nonce wipe and store()
memwipe used to clear sensitive rct::key material from m_multisig_k
Potential nonce-reuse / private-key-leak class issue in threshold multisig
Evidence from the diff
The commit refactors wallet2’s multisig nonce erasure. It introduces clear_multisig_k_and_store(), which memwipes m_transfers[idx].m_multisig_k and calls store() before any signed/partial txset is exposed. In save_multisig_tx(), the old inline wipe is removed and clear_multisig_k_and_store() is called after encryption but before returning the ciphertext. In sign_multisig_tx(), the function now works on a local copy (exported_txs), defers updating m_tx_keys/m_additional_tx_keys until after nonce erasure and storage, and only assigns the result back to the caller-provided exported_txs_inout after clear_multisig_k_and_store() succeeds. This closes a window where a partially-signed txset could be returned while the corresponding one-time nonces still resided in the wallet cache, potentially enabling nonce reuse or key-recovery attacks in a multisig setting.
Changed components
src/wallet/wallet2.cppsrc/wallet/wallet2.hwallet2::save_multisig_txwallet2::sign_multisig_txwallet2::clear_multisig_k_and_storeMonero multisig walletInspect captured patch +48 / −26
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index e7b8b81..c28bfed 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -8080,14 +8080,6 @@ std::string wallet2::save_multisig_tx(multisig_tx_set txs)
{
LOG_PRINT_L0("saving " << txs.m_ptx.size() << " multisig transactions");
- // txes generated, get rid of used k values
- for (size_t n = 0; n < txs.m_ptx.size(); ++n)
- for (size_t idx: txs.m_ptx[n].construction_data.selected_transfers)
- {
- memwipe(m_transfers[idx].m_multisig_k.data(), m_transfers[idx].m_multisig_k.size() * sizeof(m_transfers[idx].m_multisig_k[0]));
- m_transfers[idx].m_multisig_k.clear();
- }
-
// zero out some data we don't want to share
for (auto &ptx: txs.m_ptx)
{
@@ -8115,6 +8107,11 @@ std::string wallet2::save_multisig_tx(multisig_tx_set txs)
}
LOG_PRINT_L2("Saving multisig unsigned tx data: " << oss.str());
std::string ciphertext = encrypt_with_view_secret_key(oss.str());
+
+ // The transaction creator has already signed, so do not expose the txset
+ // until the corresponding one-time nonce erasure is stored.
+ clear_multisig_k_and_store(txs);
+
return std::string(MULTISIG_UNSIGNED_TX_PREFIX) + ciphertext;
}
//----------------------------------------------------------------------------------------------------
@@ -8262,8 +8259,12 @@ bool wallet2::load_multisig_tx_from_file(const std::string &filename, multisig_t
return true;
}
//----------------------------------------------------------------------------------------------------
-bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector<crypto::hash> &txids)
+bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs_inout, std::vector<crypto::hash> &txids)
{
+ multisig_tx_set exported_txs = exported_txs_inout;
+ std::vector<crypto::hash> signed_txids;
+ std::vector<std::pair<crypto::hash, size_t>> signed_tx_key_indices;
+
THROW_WALLET_EXCEPTION_IF(exported_txs.m_ptx.empty(), error::wallet_internal_error, "No tx found");
const crypto::public_key local_signer = get_multisig_signer_public_key();
@@ -8277,8 +8278,6 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector<crypto
THROW_WALLET_EXCEPTION_IF(frozen(exported_txs),
error::wallet_internal_error, "Will not sign multisig tx containing frozen outputs")
- txids.clear();
-
// The 'exported_txs' contains a set of different transactions for the multisig group to try to sign. Each of those
// transactions has a set of 'signing attempts' corresponding to all the possible signing groups within the multisig.
// - Here, we will partially sign as many of those signing attempts as possible, for each proposed transaction.
@@ -8388,25 +8387,26 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector<crypto
"Unable to finalize the transaction: the ignore sets for these tx attempts seem to be malformed.");
const crypto::hash txid = get_transaction_hash(ptx.tx);
if (store_tx_info())
- {
- m_tx_keys[txid] = ptx.tx_key;
- m_additional_tx_keys[txid] = ptx.additional_tx_keys;
- }
- txids.push_back(txid);
+ signed_tx_key_indices.emplace_back(txid, n);
+ signed_txids.push_back(txid);
}
}
- // signatures generated, get rid of any unused k values (must do export_multisig() to make more tx attempts with the
- // inputs in the transactions worked on here)
- for (size_t n = 0; n < exported_txs.m_ptx.size(); ++n)
- for (size_t idx: exported_txs.m_ptx[n].construction_data.selected_transfers)
- {
- memwipe(m_transfers[idx].m_multisig_k.data(), m_transfers[idx].m_multisig_k.size() * sizeof(m_transfers[idx].m_multisig_k[0]));
- m_transfers[idx].m_multisig_k.clear();
- }
+ exported_txs.m_signers.insert(local_signer);
- exported_txs.m_signers.insert(get_multisig_signer_public_key());
+ // Do not expose signatures until all nonce material for the selected inputs
+ // has been erased from the wallet cache.
+ clear_multisig_k_and_store(exported_txs);
+ for (const auto &entry: signed_tx_key_indices)
+ {
+ const auto &ptx = exported_txs.m_ptx[entry.second];
+ m_tx_keys[entry.first] = ptx.tx_key;
+ m_additional_tx_keys[entry.first] = ptx.additional_tx_keys;
+ }
+
+ exported_txs_inout = std::move(exported_txs);
+ txids = std::move(signed_txids);
return true;
}
//----------------------------------------------------------------------------------------------------
@@ -14410,6 +14410,27 @@ void wallet2::get_multisig_k(size_t idx, const std::unordered_set<rct::key> &use
THROW_WALLET_EXCEPTION(tools::error::multisig_export_needed);
}
//----------------------------------------------------------------------------------------------------
+void wallet2::clear_multisig_k_and_store(const multisig_tx_set &txs)
+{
+ // Must succeed before any txset produced with these nonces is exposed.
+ bool changed = false;
+ for (const auto &ptx: txs.m_ptx)
+ {
+ for (size_t idx: ptx.construction_data.selected_transfers)
+ {
+ std::vector<rct::key> &multisig_k = m_transfers[idx].m_multisig_k;
+ if (multisig_k.empty())
+ continue;
+ memwipe(multisig_k.data(), multisig_k.size() * sizeof(multisig_k[0]));
+ multisig_k.clear();
+ changed = true;
+ }
+ }
+
+ if (changed)
+ store();
+}
+//----------------------------------------------------------------------------------------------------
rct::multisig_kLRki wallet2::get_multisig_kLRki(size_t n, const rct::key &k) const
{
CHECK_AND_ASSERT_THROW_MES(n < m_transfers.size(), "Bad m_transfers index");
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index eee9eb7..c35ecd1 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -919,7 +919,7 @@ private:
bool load_multisig_tx(cryptonote::blobdata blob, multisig_tx_set &exported_txs, std::function<bool(const multisig_tx_set&)> accept_func = NULL);
bool load_multisig_tx_from_file(const std::string &filename, multisig_tx_set &exported_txs, std::function<bool(const multisig_tx_set&)> accept_func = NULL);
bool sign_multisig_tx_from_file(const std::string &filename, std::vector<crypto::hash> &txids, std::function<bool(const multisig_tx_set&)> accept_func);
- bool sign_multisig_tx(multisig_tx_set &exported_txs, std::vector<crypto::hash> &txids);
+ bool sign_multisig_tx(multisig_tx_set &exported_txs_inout, std::vector<crypto::hash> &txids);
bool sign_multisig_tx_to_file(multisig_tx_set &exported_txs, const std::string &filename, std::vector<crypto::hash> &txids);
std::vector<pending_tx> create_unmixable_sweep_transactions();
void discard_unmixable_outputs();
@@ -1578,6 +1578,7 @@ private:
rct::multisig_kLRki get_multisig_composite_kLRki(size_t n, const std::unordered_set<crypto::public_key> &ignore_set, std::unordered_set<rct::key> &used_L, std::unordered_set<rct::key> &new_used_L) const;
rct::multisig_kLRki get_multisig_kLRki(size_t n, const rct::key &k) const;
void get_multisig_k(size_t idx, const std::unordered_set<rct::key> &used_L, rct::key &nonce);
+ void clear_multisig_k_and_store(const multisig_tx_set &txs);
std::deque<crypto::public_key> multisig_available_signers() const;
std::vector<std::unordered_set<crypto::public_key>> multisig_attempt_ignore_sets() const;
void update_multisig_rescan_info(const std::vector<std::vector<rct::key>> &multisig_k, const std::vector<std::vector<tools::wallet2::multisig_info>> &info, size_t n);
Why this scored 65/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.