Use SecureString for mnemonic phrase.

Cargo.lock

@@ -882,6 +882,7 @@ dependencies = [
  "zcash_note_encryption",
  "zcash_primitives",
  "zcash_proofs",
+ "zeroize",
 ]
 
 [[package]]

Cargo.toml

@@ -48,6 +48,7 @@ zcash_note_encryption = "0.0"
 zcash_primitives = "0.5"
 zcash_proofs = "0.5"
 ed25519-zebra = "2.2.0"
+zeroize = "1.4.2"
 
 # Metrics
 hyper = { version = "=0.14.2", default-features = false, features = ["server", "tcp", "http1"] }

src/rust/src/zip339_ffi.rs

@@ -4,6 +4,7 @@ use std::{
     ffi::{CStr, CString},
     ptr, slice,
 };
+use zeroize::Zeroize;
 
 use zcash_primitives::zip339;
 
@@ -63,7 +64,9 @@ pub extern "C" fn zip339_free_phrase(phrase: *const c_char) {
     if !phrase.is_null() {
         unsafe {
             // It is correct to cast away const here; the memory is not actually immutable.
-            CString::from_raw(phrase as *mut c_char);
+            CString::from_raw(phrase as *mut c_char)
+                .into_bytes()
+                .zeroize();
         }
     }
 }

src/serialize.h

@@ -65,7 +65,7 @@ inline T* NCONST_PTR(const T* val)
     return const_cast<T*>(val);
 }
 
-/** 
+/**
  * Get begin pointer of vector (non-const version).
  * @note These functions avoid the undefined case of indexing into an empty
  * vector, as well as that of indexing after the end of the vector.
@@ -197,11 +197,11 @@ enum
 #define READWRITE(obj)      (::SerReadWrite(s, (obj), ser_action))
 #define READWRITEMANY(...)      (::SerReadWriteMany(s, ser_action, __VA_ARGS__))
 
-/** 
+/**
  * Implement two methods, "Serialize" and "Unserialize", for serializable objects. These are
  * actually wrappers over the "SerializationOp" template method, which implements the body of each
  * classes' serialization code. Adding "ADD_SERIALIZE_METHODS" in the body of the class causes these
- * wrapper methods to be added as members. 
+ * wrapper methods to be added as members.
  */
 #define ADD_SERIALIZE_METHODS                                         \
     template<typename Stream>                                         \
@@ -324,16 +324,16 @@ uint64_t ReadCompactSize(Stream& is)
  * sure the encoding is one-to-one, one is subtracted from all but the last digit.
  * Thus, the byte sequence a[] with length len, where all but the last byte
  * has bit 128 set, encodes the number:
- * 
+ *
  *  (a[len-1] & 0x7F) + sum(i=1..len-1, 128^i*((a[len-i-1] & 0x7F)+1))
- * 
+ *
  * Properties:
  * * Very small (0-127: 1 byte, 128-16511: 2 bytes, 16512-2113663: 3 bytes)
  * * Every integer has exactly one encoding
  * * Encoding does not depend on size of original integer type
  * * No redundancy: every (infinite) byte sequence corresponds to a list
  *   of encoded integers.
- * 
+ *
  * 0:         [0x00]  256:        [0x81 0x00]
  * 1:         [0x01]  16383:      [0xFE 0x7F]
  * 127:       [0x7F]  16384:      [0xFF 0x00]
@@ -394,7 +394,7 @@ I ReadVarInt(Stream& is)
 #define COMPACTSIZE(obj) REF(CCompactSize(REF(obj)))
 #define LIMITED_STRING(obj,n) REF(LimitedString< n >(REF(obj)))
 
-/** 
+/**
  * Wrapper for serializing arrays and POD.
  */
 class CFlatData
@@ -510,8 +510,11 @@ CVarInt<I> WrapVarInt(I& n) { return CVarInt<I>(n); }
 /**
  *  string
  */
-template<typename Stream, typename C> void Serialize(Stream& os, const std::basic_string<C>& str);
-template<typename Stream, typename C> void Unserialize(Stream& is, std::basic_string<C>& str);
+template<typename Stream, typename C, typename T, typename A>
+void Serialize(Stream& os, const std::basic_string<C>& str);
+
+template<typename Stream, typename C, typename T, typename A>
+void Unserialize(Stream& is, std::basic_string<C>& str);
 
 /**
  * prevector
@@ -625,16 +628,16 @@ inline void Unserialize(Stream& is, T& a)
 /**
  * string
  */
-template<typename Stream, typename C>
-void Serialize(Stream& os, const std::basic_string<C>& str)
+template<typename Stream, typename C, typename T, typename A>
+void Serialize(Stream& os, const std::basic_string<C, T, A>& str)
 {
     WriteCompactSize(os, str.size());
     if (!str.empty())
         os.write((char*)&str[0], str.size() * sizeof(str[0]));
 }
 
-template<typename Stream, typename C>
-void Unserialize(Stream& is, std::basic_string<C>& str)
+template<typename Stream, typename C, typename T, typename A>
+void Unserialize(Stream& is, std::basic_string<C, T, A>& str)
 {
     unsigned int nSize = ReadCompactSize(is);
     str.resize(nSize);

src/utiltest.cpp

@@ -316,7 +316,7 @@ void RegtestDeactivateNU5() {
 }
 
 libzcash::SaplingExtendedSpendingKey GetTestMasterSaplingSpendingKey() {
-    std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
+    SecureString mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
     MnemonicSeed seed(English, mnemonic);
     return libzcash::SaplingExtendedSpendingKey::Master(seed);
 }

src/wallet/gtest/test_wallet_zkeys.cpp

@@ -32,7 +32,7 @@ TEST(WalletZkeysTest, StoreAndLoadSaplingZkeys) {
     EXPECT_ANY_THROW(wallet.GenerateNewLegacySaplingZKey());
 
     // Load the all-zeroes seed
-    std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
+    SecureString mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
     MnemonicSeed seed(English, mnemonic);
     // The legacy seed used to be automatically derived from randomness; since
     // non-mnemonic random generation has been removed we just use the
@@ -433,7 +433,7 @@ TEST(WalletZkeysTest, WriteCryptedSaplingZkeyDirectToDb) {
     ASSERT_FALSE(wallet.HaveMnemonicSeed());
 
     // Load the all-zeroes seed as the legacy seed
-    std::string mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
+    SecureString mnemonic("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
     MnemonicSeed seed(English, mnemonic);
     wallet.LoadMnemonicSeed(seed);
     ASSERT_TRUE(wallet.HaveMnemonicSeed());

src/wallet/rpcwallet.cpp

@@ -1763,7 +1763,7 @@ UniValue walletconfirmbackup(const UniValue& params, bool fHelp)
 
     EnsureWalletIsUnlocked();
 
-    auto strMnemonicPhrase = params[0].get_str();
+    SecureString strMnemonicPhrase(params[0].get_str());
     boost::erase_all(strMnemonicPhrase, "\"");
     boost::trim(strMnemonicPhrase);
     if (strMnemonicPhrase.length() > 0) {

src/wallet/wallet.cpp

@@ -2355,7 +2355,7 @@ bool CWallet::SetCryptedMnemonicSeed(const uint256& seedFp, const std::vector<un
     return false;
 }
 
-bool CWallet::VerifyMnemonicSeed(std::string mnemonic) {
+bool CWallet::VerifyMnemonicSeed(const SecureString& mnemonic) {
     LOCK(cs_wallet);
 
     auto seed = GetMnemonicSeed();

src/wallet/wallet.h

@@ -1305,7 +1305,7 @@ public:
     bool SetCryptedMnemonicSeed(const uint256& seedFp, const std::vector<unsigned char> &vchCryptedSecret);
     /* Checks the wallet's seed against the specified mnemonic, and marks the
      * wallet's seed as having been backed up if the phrases match. */
-    bool VerifyMnemonicSeed(std::string mnemonic);
+    bool VerifyMnemonicSeed(const SecureString& mnemonic);
     bool MnemonicVerified();
 
     /* Set the current HD seed, without saving it to disk (used by LoadWallet) */

src/zcash/address/zip32.cpp

@@ -28,7 +28,7 @@ MnemonicSeed MnemonicSeed::Random(uint32_t bip44CoinType, Language language, siz
         std::vector<unsigned char> entropy(entropyLen, 0);
         GetRandBytes(entropy.data(), entropyLen);
         const char* phrase = zip339_entropy_to_phrase(language, entropy.data(), entropyLen);
-        std::string mnemonic(phrase);
+        SecureString mnemonic(phrase);
         zip339_free_phrase(phrase);
         MnemonicSeed seed(language, mnemonic);
 

src/zcash/address/zip32.h

@@ -66,21 +66,18 @@ public:
 class MnemonicSeed: public HDSeed {
 private:
     Language language;
-    std::string mnemonic;
+    SecureString mnemonic;
 
     MnemonicSeed() {}
 
-    void Init() {
-        unsigned char buf[64];
-        zip339_phrase_to_seed(language, mnemonic.c_str(), &buf);
-        seed.assign(buf, std::end(buf));
+    void SetSeedFromMnemonic() {
+        seed.resize(64);
+        zip339_phrase_to_seed(language, mnemonic.c_str(), (uint8_t (*)[64])seed.data());
     }
 
 public:
-    MnemonicSeed(Language languageIn, std::string& mnemonicIn): language(languageIn), mnemonic(mnemonicIn) {
-        unsigned char buf[64];
-        zip339_phrase_to_seed(languageIn, mnemonicIn.c_str(), &buf);
-        seed.assign(buf, std::end(buf));
+    MnemonicSeed(Language languageIn, SecureString mnemonicIn): language(languageIn), mnemonic(mnemonicIn) {
+        SetSeedFromMnemonic();
     }
 
     /**
@@ -127,7 +124,7 @@ public:
             READWRITE(language0);
             READWRITE(mnemonic);
             language = (Language) language0;
-            Init();
+            SetSeedFromMnemonic();
         } else {
             uint32_t language0 = (uint32_t) language;
 
@@ -147,7 +144,7 @@ public:
         return language;
     }
 
-    const std::string GetMnemonic() const {
+    const SecureString& GetMnemonic() const {
         return mnemonic;
     }