multisig: reject mismatched key image component count
What changed, and why it matters
This Monero commit hardens the multisig wallet code that combines partial key images from multiple signers. Previously, the code could silently build an incomplete or wrong composite key image if it received too few, too many, or duplicate partial components. The patch now rejects mismatched counts and duplicate/identity partial key images during import, and validates imported multisig data before it replaces the wallet's trusted state. A bad key image could let a malicious or buggy signer interfere with spending or detection of funds, so the change is a defensive security fix.
Treat this as a security-hardening fix and include it in the next release. Users running multisig wallets should upgrade. Review whether prior versions could produce or accept malformed composite key images in multisig import/rescan workflows.
Security signals we found
Defensive validation added to multisig key-image composition
Rejection of duplicate and identity partial key images
Rejection of reused signing nonces (m_L) in imported multisig data
Pre-installation validation of imported multisig rescan info
Subgroup validation already present, retained
Evidence from the diff
The patch modifies generate_multisig_composite_key_image to require that the number of distinct key-image components used equals the expected combinatorial count for the multisig configuration (combinations_count(num_signers - threshold + 1, num_signers)). It also adds num_signers and threshold parameters to that function. wallet2::import_multisig now checks for duplicate signing nonces (m_L), duplicate/identity partial key images, subgroup membership, and validates every output’s composite key image before installing the rescan state. update_multisig_rescan_info now validates the candidate info before overwriting td.m_multisig_info. Tests are added/updated to cover these checks.
Changed components
src/multisig/multisig.cppsrc/multisig/multisig.hsrc/wallet/wallet2.cppsrc/wallet/wallet2.htests/core_tests/multisig.cpptests/unit_tests/multisig.cppInspect captured patch +133 / −20
### src/multisig/multisig.cpp
@@ -33,6 +33,7 @@
#include "include_base_utils.h"
#include "multisig.h"
#include "ringct/rctOps.h"
+#include "common/combinator.h"
#include <algorithm>
#include <unordered_map>
@@ -90,6 +91,8 @@ namespace multisig
const std::vector<crypto::public_key> &additional_tx_public_keys,
std::size_t real_output_index,
const std::vector<crypto::key_image> &pkis,
+ std::size_t num_signers,
+ std::size_t threshold,
crypto::key_image &ki)
{
// create a multisig partial key image
@@ -134,6 +137,12 @@ namespace multisig
// - if 'pkis' (the other participants' KI components) is missing some components
// then 'ki' will not be complete
+ // a right-sized 'used' doesn't prove 'ki' is correct, but a wrong-sized one does prove it's
+ // incomplete or was built from duplicate/extraneous components, so reject that case here
+ // instead of silently returning a bad key image.
+ const std::size_t expected_num_key_image_components = tools::combinations_count(num_signers - threshold + 1, num_signers);
+ if (used.size() != expected_num_key_image_components) return false;
+
return true;
}
//----------------------------------------------------------------------------------------------------------------------
### src/multisig/multisig.h
@@ -65,5 +65,7 @@ namespace multisig
const std::vector<crypto::public_key> &additional_tx_public_keys,
std::size_t real_output_index,
const std::vector<crypto::key_image> &pkis,
+ std::size_t num_signers,
+ std::size_t threshold,
crypto::key_image &ki);
} //namespace multisig
### src/wallet/wallet2.cpp
@@ -14742,6 +14742,12 @@ rct::multisig_kLRki wallet2::get_multisig_composite_kLRki(size_t n, const std::u
}
//----------------------------------------------------------------------------------------------------
crypto::key_image wallet2::get_multisig_composite_key_image(size_t n) const
+{
+ CHECK_AND_ASSERT_THROW_MES(n < m_transfers.size(), "Bad output index");
+ return get_multisig_composite_key_image(n, m_transfers[n].m_multisig_info);
+}
+//----------------------------------------------------------------------------------------------------
+crypto::key_image wallet2::get_multisig_composite_key_image(size_t n, const std::vector<multisig_info> &infos) const
{
CHECK_AND_ASSERT_THROW_MES(n < m_transfers.size(), "Bad output index");
@@ -14750,10 +14756,10 @@ crypto::key_image wallet2::get_multisig_composite_key_image(size_t n) const
const std::vector<crypto::public_key> additional_tx_keys = cryptonote::get_additional_tx_pub_keys_from_extra(td.m_tx);
crypto::key_image ki;
std::vector<crypto::key_image> pkis;
- for (const auto &info: td.m_multisig_info)
+ for (const auto &info: infos)
for (const auto &pki: info.m_partial_key_images)
pkis.push_back(pki);
- bool r = multisig::generate_multisig_composite_key_image(get_account().get_keys(), m_subaddresses, td.get_public_key(), tx_key, additional_tx_keys, td.m_internal_output_index, pkis, ki);
+ bool r = multisig::generate_multisig_composite_key_image(get_account().get_keys(), m_subaddresses, td.get_public_key(), tx_key, additional_tx_keys, td.m_internal_output_index, pkis, m_multisig_signers.size(), m_multisig_threshold, ki);
THROW_WALLET_EXCEPTION_IF(!r, error::wallet_internal_error, "Failed to generate key image");
return ki;
}
@@ -14915,14 +14921,22 @@ void wallet2::update_multisig_rescan_info(const std::vector<std::vector<rct::key
MDEBUG("update_multisig_rescan_info: updating index " << n);
transfer_details &td = m_transfers[n];
- td.m_multisig_info.clear();
+
+ // validate the candidate before touching td.m_multisig_info, so a rejected candidate will never
+ // overwrite installed info. only catches count mismatches (duplicate/missing components), not
+ // a well-formed component that's just wrong for this output.
+ std::vector<multisig_info> new_info;
+ new_info.reserve(info.size());
for (const auto &pi: info)
{
CHECK_AND_ASSERT_THROW_MES(n < pi.size(), "Bad pi size");
- td.m_multisig_info.push_back(pi[n]);
+ new_info.push_back(pi[n]);
}
+ const crypto::key_image new_key_image = get_multisig_composite_key_image(n, new_info);
+
+ td.m_multisig_info = std::move(new_info);
m_key_images.erase(td.m_key_image);
- td.m_key_image = get_multisig_composite_key_image(n);
+ td.m_key_image = new_key_image;
td.m_key_image_known = true;
td.m_key_image_request = false;
td.m_key_image_partial = false;
@@ -14942,7 +14956,7 @@ size_t wallet2::import_multisig(std::vector<cryptonote::blobdata> blobs, bool re
std::vector<std::vector<tools::wallet2::multisig_info>> info;
std::unordered_set<crypto::public_key> seen;
- const size_t expected_n_partial_key_images = get_account().get_multisig_keys().size();
+ const uint64_t expected_n_partial_key_images = num_priv_multisig_keys_post_setup(m_multisig_threshold, m_multisig_signers.size());
const size_t expected_n_lr = tools::combinations_count(m_multisig_signers.size() - m_multisig_threshold, m_multisig_signers.size() - 1)
* multisig::signing::kAlphaComponents;
@@ -14999,14 +15013,19 @@ size_t wallet2::import_multisig(std::vector<cryptonote::blobdata> blobs, bool re
CHECK_AND_ASSERT_THROW_MES(e.m_LR.size() == expected_n_lr,
"Multisig info has an unexpected number of signing nonces");
+ std::unordered_set<rct::key> seen_L;
for (const auto &lr: e.m_LR)
{
CHECK_AND_ASSERT_THROW_MES(rct::isInMainSubgroup(lr.m_L), "Multisig value is not in the main subgroup");
CHECK_AND_ASSERT_THROW_MES(rct::isInMainSubgroup(lr.m_R), "Multisig value is not in the main subgroup");
+ CHECK_AND_ASSERT_THROW_MES(seen_L.insert(lr.m_L).second, "Multisig info reuses a signing nonce");
}
+ std::unordered_set<crypto::key_image> seen_ki;
for (const auto &ki: e.m_partial_key_images)
{
CHECK_AND_ASSERT_THROW_MES(rct::isInMainSubgroup(rct::ki2rct(ki)), "Multisig partial key image is not in the main subgroup");
+ CHECK_AND_ASSERT_THROW_MES(rct::ki2rct(ki) != rct::identity(), "Multisig partial key image must not be the identity element");
+ CHECK_AND_ASSERT_THROW_MES(seen_ki.insert(ki).second, "Multisig info has a duplicate partial key image");
}
}
@@ -15031,13 +15050,11 @@ size_t wallet2::import_multisig(std::vector<cryptonote::blobdata> blobs, bool re
if (n_outputs == 0)
return 0;
- // check signers are consistent
+ // check signers are members of this wallet
for (const auto &pi: info)
{
CHECK_AND_ASSERT_THROW_MES(std::find(m_multisig_signers.begin(), m_multisig_signers.end(), pi[0].m_signer) != m_multisig_signers.end(),
"Signer is not a member of this multisig wallet");
- for (size_t n = 1; n < n_outputs; ++n)
- CHECK_AND_ASSERT_THROW_MES(pi[n].m_signer == pi[0].m_signer, "Mismatched signers in imported multisig info");
}
// trim data we don't have info for from all participants
@@ -15050,6 +15067,18 @@ size_t wallet2::import_multisig(std::vector<cryptonote::blobdata> blobs, bool re
std::sort(info.begin(), info.end(), [](const std::vector<tools::wallet2::multisig_info> &i0, const std::vector<tools::wallet2::multisig_info> &i1){ return memcmp(&i0[0].m_signer, &i1[0].m_signer, sizeof(i0[0].m_signer)) < 0; });
}
+ // validate every output before installing rescan state or detaching the chain. will only catch
+ // count mismatches (missing/duplicate components), not a well-formed component that's just
+ // wrong for this output.
+ for (size_t n = 0; n < n_outputs && n < m_transfers.size(); ++n)
+ {
+ std::vector<multisig_info> candidate;
+ candidate.reserve(info.size());
+ for (const auto &pi: info)
+ candidate.push_back(pi[n]);
+ get_multisig_composite_key_image(n, candidate);
+ }
+
// wipe prior pending rescan state and install its replacement only after full validation
for (auto &v: m_multisig_rescan_k)
memwipe(v.data(), v.size() * sizeof(v[0]));
### src/wallet/wallet2.h
@@ -1610,6 +1610,7 @@ namespace tools
void scan_output(const cryptonote::transaction &tx, bool miner_tx, const crypto::public_key &tx_pub_key, size_t i, tx_scan_info_t &tx_scan_info, int &num_vouts_received, std::unordered_map<cryptonote::subaddress_index, uint64_t> &tx_money_got_in_outs, std::vector<size_t> &outs, bool pool);
void trim_hashchain();
crypto::key_image get_multisig_composite_key_image(size_t n) const;
+ crypto::key_image get_multisig_composite_key_image(size_t n, const std::vector<multisig_info> &infos) const;
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);
### tests/core_tests/multisig.cpp
@@ -253,13 +253,13 @@ bool gen_multisig_tx_validation_base::generate_with(std::vector<test_event_entry
for (size_t msidx = 0; msidx < total; ++msidx)
for (size_t n = 0; n < account_ki[msidx][tdidx].size(); ++n)
pkis.push_back(account_ki[msidx][tdidx][n]);
- r = multisig::generate_multisig_composite_key_image(miner_account[0].get_keys(), subaddresses, output_pub_key[tdidx], tx_pub_key[tdidx], additional_tx_keys, 0, pkis, (crypto::key_image&)kLRki.ki);
+ r = multisig::generate_multisig_composite_key_image(miner_account[0].get_keys(), subaddresses, output_pub_key[tdidx], tx_pub_key[tdidx], additional_tx_keys, 0, pkis, total, threshold, (crypto::key_image&)kLRki.ki);
CHECK_AND_ASSERT_MES(r, false, "Failed to generate composite key image");
MDEBUG("composite ki: " << kLRki.ki);
for (size_t n = 1; n < total; ++n)
{
rct::key ki;
- r = multisig::generate_multisig_composite_key_image(miner_account[n].get_keys(), subaddresses, output_pub_key[tdidx], tx_pub_key[tdidx], additional_tx_keys, 0, pkis, (crypto::key_image&)ki);
+ r = multisig::generate_multisig_composite_key_image(miner_account[n].get_keys(), subaddresses, output_pub_key[tdidx], tx_pub_key[tdidx], additional_tx_keys, 0, pkis, total, threshold, (crypto::key_image&)ki);
CHECK_AND_ASSERT_MES(r, false, "Failed to generate composite key image");
CHECK_AND_ASSERT_MES(kLRki.ki == ki, false, "Composite key images do not match");
}
### tests/unit_tests/multisig.cpp
@@ -548,18 +548,24 @@ TEST(multisig, import_multisig_validation)
crypto::key_image valid_pki;
ASSERT_TRUE(generate_multisig_key_image(wallets[1].get_account().get_keys(), 0, fake_out_key, valid_pki));
- tools::wallet2::multisig_info::LR valid_lr;
- {
- const crypto::secret_key k = rct::rct2sk(rct::skGen());
- crypto::public_key L, R;
- generate_multisig_LR(fake_out_key, k, L, R);
- valid_lr.m_L = rct::pk2rct(L);
- valid_lr.m_R = rct::pk2rct(R);
- }
+ // each LR pair should come from its own independently drawn nonce: a real export never repeats
+ // one, since two signing attempts sharing a nonce would risk leaking the secret key
+ const auto make_lr =
+ [&]() -> tools::wallet2::multisig_info::LR
+ {
+ tools::wallet2::multisig_info::LR lr;
+ const crypto::secret_key k = rct::rct2sk(rct::skGen());
+ crypto::public_key L, R;
+ generate_multisig_LR(fake_out_key, k, L, R);
+ lr.m_L = rct::pk2rct(L);
+ lr.m_R = rct::pk2rct(R);
+ return lr;
+ };
+ const tools::wallet2::multisig_info::LR valid_lr = make_lr();
tools::wallet2::multisig_info valid_entry;
valid_entry.m_signer = other_signer;
- valid_entry.m_LR = {valid_lr, valid_lr};
+ valid_entry.m_LR = {valid_lr, make_lr()};
valid_entry.m_partial_key_images = {valid_pki};
// builds a raw multisig-info import blob (same wire format as wallet2::export_multisig()) for a single
@@ -605,3 +611,69 @@ TEST(multisig, import_multisig_validation)
EXPECT_ANY_THROW(wallets[0].import_multisig({build_blob(other_signer, bad_entry)}, false));
}
}
+
+TEST(multisig, composite_key_image_rejects_wrong_content)
+{
+ using namespace multisig;
+
+ // same 2-of-2 shape as 'import_multisig_validation': each signer holds exactly 1 multisig
+ // private key, and a composite needs combinations_count(N-M+1, N) = combinations_count(1, 2) = 2
+ // distinct key image components (1 from each signer) to be considered complete
+ const std::uint32_t M = 2, N = 2;
+ std::vector<tools::wallet2> wallets(N);
+
+ std::vector<std::string> initial_infos(wallets.size());
+ for (size_t i = 0; i < wallets.size(); ++i)
+ {
+ make_wallet(i, wallets[i]);
+ wallets[i].decrypt_keys("");
+ initial_infos[i] = wallets[i].get_multisig_first_kex_msg();
+ wallets[i].encrypt_keys("");
+ }
+
+ std::vector<std::string> intermediate_infos(wallets.size());
+ for (size_t i = 0; i < wallets.size(); ++i)
+ intermediate_infos[i] = wallets[i].make_multisig("", initial_infos, M);
+
+ multisig_account_status ms_status{wallets[0].get_multisig_status()};
+ while (!ms_status.is_ready)
+ {
+ intermediate_infos = exchange_round(wallets, intermediate_infos);
+ ms_status = wallets[0].get_multisig_status();
+ }
+
+ wallets[0].decrypt_keys("");
+ wallets[1].decrypt_keys("");
+
+ // a real one-time output address for the multisig account's main subaddress index, since
+ // (unlike generate_multisig_key_image/generate_multisig_LR) generate_multisig_composite_key_image
+ // first derives the output's base key image and requires it to actually belong to the account
+ const crypto::secret_key tx_sk = rct::rct2sk(rct::skGen());
+ crypto::public_key tx_pub_key;
+ ASSERT_TRUE(crypto::secret_key_to_public_key(tx_sk, tx_pub_key));
+ const cryptonote::account_public_address &addr = wallets[0].get_account().get_keys().m_account_address;
+ crypto::key_derivation derivation;
+ ASSERT_TRUE(crypto::generate_key_derivation(addr.m_view_public_key, tx_sk, derivation));
+ crypto::public_key out_key;
+ ASSERT_TRUE(crypto::derive_public_key(derivation, 0, addr.m_spend_public_key, out_key));
+
+ std::unordered_map<crypto::public_key, cryptonote::subaddress_index> subaddresses;
+ subaddresses[addr.m_spend_public_key] = {0, 0};
+
+ // a genuine, distinct component from the other signer lets the composite complete
+ crypto::key_image other_component;
+ ASSERT_TRUE(generate_multisig_key_image(wallets[1].get_account().get_keys(), 0, out_key, other_component));
+
+ crypto::key_image ki_good;
+ EXPECT_TRUE(generate_multisig_composite_key_image(wallets[0].get_account().get_keys(), subaddresses, out_key,
+ tx_pub_key, {}, 0, {other_component}, N, M, ki_good));
+
+ // right count, wrong content: a duplicate of the local component instead of the other signer's.
+ // proves the count check catches this collision, not wrong-but-distinct content in general.
+ crypto::key_image local_component;
+ ASSERT_TRUE(generate_multisig_key_image(wallets[0].get_account().get_keys(), 0, out_key, local_component));
+
+ crypto::key_image ki_bad;
+ EXPECT_FALSE(generate_multisig_composite_key_image(wallets[0].get_account().get_keys(), subaddresses, out_key,
+ tx_pub_key, {}, 0, {local_component}, N, M, ki_bad));
+}Why this scored 56/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.