From 613071f4fa6a823834198ae8d180a5af72e71ded Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 8 Apr 2020 16:35:28 +0000 Subject: [PATCH] use memwipe on secret k/alpha values Reported by UkoeHB_ and sarang --- src/crypto/crypto.cpp | 6 ++++++ src/multisig/multisig.cpp | 7 ++++--- src/ringct/rctSigs.cpp | 5 ++++- src/ringct/rctTypes.h | 3 +++ src/wallet/wallet2.cpp | 16 ++++++++++------ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/crypto/crypto.cpp b/src/crypto/crypto.cpp index 0ec992de9..8a03f28bb 100644 --- a/src/crypto/crypto.cpp +++ b/src/crypto/crypto.cpp @@ -294,6 +294,7 @@ namespace crypto { sc_mulsub(&sig.r, &sig.c, &unwrap(sec), &k); if (!sc_isnonzero((const unsigned char*)sig.r.data)) goto try_again; + memwipe(&k, sizeof(k)); } bool crypto_ops::check_signature(const hash &prefix_hash, const public_key &pub, const signature &sig) { @@ -390,6 +391,8 @@ namespace crypto { // sig.r = k - sig.c*r sc_mulsub(&sig.r, &sig.c, &unwrap(r), &k); + + memwipe(&k, sizeof(k)); } bool crypto_ops::check_tx_proof(const hash &prefix_hash, const public_key &R, const public_key &A, const boost::optional &B, const public_key &D, const signature &sig) { @@ -560,6 +563,7 @@ POP_WARNINGS random_scalar(sig[i].c); random_scalar(sig[i].r); if (ge_frombytes_vartime(&tmp3, &*pubs[i]) != 0) { + memwipe(&k, sizeof(k)); local_abort("invalid pubkey"); } ge_double_scalarmult_base_vartime(&tmp2, &sig[i].c, &tmp3, &sig[i].r); @@ -573,6 +577,8 @@ POP_WARNINGS hash_to_scalar(buf.get(), rs_comm_size(pubs_count), h); sc_sub(&sig[sec_index].c, &h, &sum); sc_mulsub(&sig[sec_index].r, &sig[sec_index].c, &unwrap(sec), &k); + + memwipe(&k, sizeof(k)); } bool crypto_ops::check_ring_signature(const hash &prefix_hash, const key_image &image, diff --git a/src/multisig/multisig.cpp b/src/multisig/multisig.cpp index 999894db0..70a4c1c8e 100644 --- a/src/multisig/multisig.cpp +++ b/src/multisig/multisig.cpp @@ -82,6 +82,7 @@ namespace cryptonote { rct::key sk = rct::scalarmultKey(rct::pk2rct(k), rct::sk2rct(blinded_skey)); crypto::secret_key msk = get_multisig_blinded_secret_key(rct::rct2sk(sk)); + memwipe(&sk, sizeof(sk)); multisig_keys.push_back(msk); sc_add(spend_skey.bytes, spend_skey.bytes, (const unsigned char*)msk.data); } @@ -126,10 +127,10 @@ namespace cryptonote //----------------------------------------------------------------- crypto::secret_key generate_multisig_view_secret_key(const crypto::secret_key &skey, const std::vector &skeys) { - rct::key view_skey = rct::sk2rct(get_multisig_blinded_secret_key(skey)); + crypto::secret_key view_skey = get_multisig_blinded_secret_key(skey); for (const auto &k: skeys) - sc_add(view_skey.bytes, view_skey.bytes, rct::sk2rct(k).bytes); - return rct::rct2sk(view_skey); + sc_add((unsigned char*)&view_skey, rct::sk2rct(view_skey).bytes, rct::sk2rct(k).bytes); + return view_skey; } //----------------------------------------------------------------- crypto::public_key generate_multisig_M_N_spend_public_key(const std::vector &pkeys) diff --git a/src/ringct/rctSigs.cpp b/src/ringct/rctSigs.cpp index a7b265d63..2e3e7007e 100644 --- a/src/ringct/rctSigs.cpp +++ b/src/ringct/rctSigs.cpp @@ -29,6 +29,7 @@ // THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "misc_log_ex.h" +#include "misc_language.h" #include "common/perf_timer.h" #include "common/threadpool.h" #include "common/util.h" @@ -108,6 +109,7 @@ namespace rct { //Borromean (c.f. gmax/andytoshi's paper) boroSig genBorromean(const key64 x, const key64 P1, const key64 P2, const bits indices) { key64 L[2], alpha; + auto wiper = epee::misc_utils::create_scope_leave_handler([&](){memwipe(alpha, sizeof(alpha));}); key c; int naught = 0, prime = 0, ii = 0, jj=0; boroSig bb; @@ -190,6 +192,7 @@ namespace rct { vector Ip(dsRows); rv.II = keyV(dsRows); keyV alpha(rows); + auto wiper = epee::misc_utils::create_scope_leave_handler([&](){memwipe(alpha.data(), alpha.size() * sizeof(alpha[0]));}); keyV aG(rows); rv.ss = keyM(cols, aG); keyV aHP(dsRows); @@ -548,7 +551,7 @@ namespace rct { subKeys(M[i][1], pubs[i].mask, Cout); } mgSig result = MLSAG_Gen(message, M, sk, kLRki, mscout, index, rows, hwdev); - memwipe(&sk[0], sizeof(key)); + memwipe(sk.data(), sk.size() * sizeof(key)); return result; } diff --git a/src/ringct/rctTypes.h b/src/ringct/rctTypes.h index bf4b7b4aa..9b7f26a02 100644 --- a/src/ringct/rctTypes.h +++ b/src/ringct/rctTypes.h @@ -48,6 +48,7 @@ extern "C" { #include "hex.h" #include "span.h" +#include "memwipe.h" #include "serialization/vector.h" #include "serialization/debug_archive.h" #include "serialization/binary_archive.h" @@ -106,6 +107,8 @@ namespace rct { key L; key R; key ki; + + ~multisig_kLRki() { memwipe(&k, sizeof(k)); } }; struct multisig_out { diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index f05a67315..8a794ba6c 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -4801,6 +4801,7 @@ std::string wallet2::make_multisig(const epee::wipeable_string &password, std::vector multisig_keys; rct::key spend_pkey = rct::identity(); rct::key spend_skey; + auto wiper = epee::misc_utils::create_scope_leave_handler([&](){memwipe(&spend_skey, sizeof(spend_skey));}); std::vector multisig_signers; // decrypt keys @@ -6411,7 +6412,7 @@ void wallet2::commit_tx(pending_tx& ptx) // tx generated, get rid of used k values for (size_t idx: ptx.selected_transfers) - m_transfers[idx].m_multisig_k.clear(); + memwipe(m_transfers[idx].m_multisig_k.data(), m_transfers[idx].m_multisig_k.size() * sizeof(m_transfers[idx].m_multisig_k[0])); //fee includes dust if dust policy specified it. LOG_PRINT_L1("Transaction successfully sent. <" << txid << ">" << ENDL @@ -6853,13 +6854,13 @@ std::string wallet2::save_multisig_tx(multisig_tx_set txs) // 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) - m_transfers[idx].m_multisig_k.clear(); + memwipe(m_transfers[idx].m_multisig_k.data(), m_transfers[idx].m_multisig_k.size() * sizeof(m_transfers[idx].m_multisig_k[0])); // zero out some data we don't want to share for (auto &ptx: txs.m_ptx) { for (auto &e: ptx.construction_data.sources) - e.multisig_kLRki.k = rct::zero(); + memwipe(&e.multisig_kLRki.k, sizeof(e.multisig_kLRki.k)); } for (auto &ptx: txs.m_ptx) @@ -7067,10 +7068,12 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector blobs) CHECK_AND_ASSERT_THROW_MES(info.size() + 1 <= m_multisig_signers.size() && info.size() + 1 >= m_multisig_threshold, "Wrong number of multisig sources"); std::vector> k; + auto wiper = epee::misc_utils::create_scope_leave_handler([&](){memwipe(k.data(), k.size() * sizeof(k[0]));}); k.reserve(m_transfers.size()); for (const auto &td: m_transfers) k.push_back(td.m_multisig_k);