Merge pull request #5421 from nuttycom/feature/wallet_unified_addresses-mnemonic_seed_init_validation

Check the output of zip339_phrase_to_seed in MnemonicSeed initialization.

src/gtest/test_keystore.cpp

@@ -45,6 +45,23 @@ TEST(KeystoreTests, StoreAndRetrieveMnemonicSeed) {
     EXPECT_EQ(seed, seedOut.value());
 }
 
+TEST(KeystoreTests, DecodeInvalidMnemonic) {
+    SecureString mnemonic("\xff");
+    EXPECT_FALSE(MnemonicSeed::ForPhrase(Language::English, mnemonic).has_value());
+}
+
+TEST(KeystoreTests, DeserializeMnemonic) {
+    CDataStream ss0(SER_NETWORK, CLIENT_VERSION);
+    ss0 << (uint32_t)English;
+    ss0 << SecureString("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art");
+    EXPECT_NO_THROW(MnemonicSeed::Read(ss0));
+
+    CDataStream ss(SER_NETWORK, CLIENT_VERSION);
+    ss << (uint32_t)English;
+    ss << SecureString("\xff");
+    EXPECT_THROW(MnemonicSeed::Read(ss), std::ios_base::failure);
+}
+
 TEST(KeystoreTests, StoreAndRetrieveLegacyHDSeed) {
     CBasicKeyStore keyStore;
 

src/utiltest.cpp

@@ -317,7 +317,7 @@ void RegtestDeactivateNU5() {
 
 libzcash::SaplingExtendedSpendingKey GetTestMasterSaplingSpendingKey() {
     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);
+    auto seed{MnemonicSeed::ForPhrase(English, mnemonic).value()};
     return libzcash::SaplingExtendedSpendingKey::Master(seed);
 }
 

src/zcash/address/mnemonic.cpp

@@ -20,7 +20,11 @@ MnemonicSeed MnemonicSeed::Random(uint32_t bip44CoinType, Language language, siz
         const char* phrase = zip339_entropy_to_phrase(language, entropy.data(), entropyLen);
         SecureString mnemonic(phrase);
         zip339_free_phrase(phrase);
-        MnemonicSeed seed(language, mnemonic);
+
+        // The phrase returned from zip339_entropy_to_phrase should always be a
+        // valid UTF-8 string; this `.value()` unwrap will correctly throw a
+        // `std::bad_optional_access` exception if that invariant does not hold.
+        auto seed = MnemonicSeed::ForPhrase(language, mnemonic).value();
 
         // Verify that the seed data is valid entropy for unified spending keys at
         // account 0 and at both the public & private chain levels for account 0x7FFFFFFF.

src/zcash/address/mnemonic.h

@@ -12,16 +12,22 @@ private:
     Language language;
     SecureString mnemonic;
 
-    MnemonicSeed() {}
-
-    void SetSeedFromMnemonic() {
+    bool SetSeedFromMnemonic() {
         seed.resize(64);
-        zip339_phrase_to_seed(language, mnemonic.c_str(), (uint8_t (*)[64])seed.data());
+        return zip339_phrase_to_seed(language, mnemonic.c_str(), (uint8_t (*)[64])seed.data());
     }
 
+    MnemonicSeed() {}
 public:
-    MnemonicSeed(Language languageIn, SecureString mnemonicIn): language(languageIn), mnemonic(mnemonicIn) {
-        SetSeedFromMnemonic();
+    static std::optional<MnemonicSeed> ForPhrase(Language languageIn, SecureString mnemonicIn) {
+        MnemonicSeed seed;
+        seed.language = languageIn;
+        seed.mnemonic = mnemonicIn;
+        if (seed.SetSeedFromMnemonic()) {
+            return seed;
+        } else {
+            return std::nullopt;
+        }
     }
 
     /**
@@ -68,7 +74,9 @@ public:
             READWRITE(language0);
             READWRITE(mnemonic);
             language = (Language) language0;
-            SetSeedFromMnemonic();
+            if (!SetSeedFromMnemonic()) {
+                throw std::ios_base::failure("Could not interpret the mnemonic phrase as a valid UTF-8 string.");
+            }
         } else {
             uint32_t language0 = (uint32_t) language;