From 0e80f167e7a086ad51fc930f2318d07dc2bd9bb4 Mon Sep 17 00:00:00 2001 From: striderDM <51991544+StriderDM@users.noreply.github.com> Date: Tue, 23 Nov 2021 13:00:53 +0200 Subject: [PATCH] fix: seed word parsing Moved detect_language into MnemonicLanguage and made it public. Prevented a TariSeedWords object from becoming invalid in the event an invalid or inconsistent word was attempted to be pushed to it in wallet_ffi. Differentiated between an invalid word and an inconsistent word. Added word to string of WordNotFound error. Review comments --- base_layer/key_manager/src/error.rs | 4 +- base_layer/key_manager/src/mnemonic.rs | 122 ++++++++++++++----------- base_layer/wallet_ffi/src/enums.rs | 1 + base_layer/wallet_ffi/src/lib.rs | 72 +++++++++++---- 4 files changed, 127 insertions(+), 72 deletions(-) diff --git a/base_layer/key_manager/src/error.rs b/base_layer/key_manager/src/error.rs index 14b79ea058..3d67c59eb1 100644 --- a/base_layer/key_manager/src/error.rs +++ b/base_layer/key_manager/src/error.rs @@ -51,8 +51,8 @@ pub enum MnemonicError { defined natural languages" )] UnknownLanguage, - #[error("Only 2048 words for each language was selected to form Mnemonic word lists")] - WordNotFound, + #[error("Word not found: `{0}`")] + WordNotFound(String), #[error("A mnemonic word does not exist for the requested index")] IndexOutOfBounds, #[error("A problem encountered constructing a secret key from bytes or mnemonic sequence: `{0}`")] diff --git a/base_layer/key_manager/src/mnemonic.rs b/base_layer/key_manager/src/mnemonic.rs index 6e7e6ef8f3..5a65f64ac7 100644 --- a/base_layer/key_manager/src/mnemonic.rs +++ b/base_layer/key_manager/src/mnemonic.rs @@ -48,7 +48,7 @@ impl MnemonicLanguage { /// Detects the mnemonic language of a specific word by searching all defined mnemonic word lists pub fn from(mnemonic_word: &str) -> Result { let words = vec![mnemonic_word.to_string()]; - detect_language(&words) + MnemonicLanguage::detect_language(&words) } /// Returns an iterator for the MnemonicLanguage enum group to allow iteration over all defined languages @@ -77,6 +77,63 @@ impl MnemonicLanguage { MnemonicLanguage::Spanish => MNEMONIC_SPANISH_WORDS.len(), } } + + /// Detects the language of a list of words + pub fn detect_language(words: &[String]) -> Result { + let count = words.iter().len(); + match count.cmp(&1) { + Ordering::Less => { + return Err(MnemonicError::UnknownLanguage); + }, + Ordering::Equal => { + let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?; + for language in MnemonicLanguage::iterator() { + if find_mnemonic_index_from_word(word, language).is_ok() { + return Ok(*language); + } + } + return Err(MnemonicError::UnknownLanguage); + }, + Ordering::Greater => { + for word in words { + let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len()); + // detect all languages in which a word falls into + for language in MnemonicLanguage::iterator() { + if find_mnemonic_index_from_word(word, language).is_ok() { + languages.push(*language); + } + } + // check if at least one of the languages is consistent for all other words against languages + // yielded from the initial word for this iteration + for language in languages { + let mut consistent = true; + for compare in words { + if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() { + consistent = false; + } + } + if consistent { + return Ok(language); + } + } + } + }, + } + + Err(MnemonicError::UnknownLanguage) + } + + /// Finds the first word that is inconsistent in the set of words for a specified language. If multiple + /// inconsistent words exist it will need to be run repeatedly, removing the inconsistent word after each iteration + pub fn find_first_inconsistent_word(words: &[String], language: &MnemonicLanguage) -> Option { + for word in words { + match find_mnemonic_index_from_word(word, language).is_err() { + true => return Some(word.to_string()), + false => {}, + }; + } + None + } } /// Finds and returns the index of a specific word in a mnemonic word list defined by the specified language @@ -106,7 +163,7 @@ fn find_mnemonic_index_from_word(word: &str, language: &MnemonicLanguage) -> Res } match search_result { Ok(v) => Ok(v), - Err(_err) => Err(MnemonicError::WordNotFound), + Err(_err) => Err(MnemonicError::WordNotFound(word.to_string())), } } @@ -154,54 +211,10 @@ pub fn from_bytes(bytes: Vec, language: &MnemonicLanguage) -> Result Result { - let count = words.iter().len(); - match count.cmp(&1) { - Ordering::Less => { - return Err(MnemonicError::UnknownLanguage); - }, - Ordering::Equal => { - let word = words.get(0).ok_or(MnemonicError::EncodeInvalidLength)?; - for language in MnemonicLanguage::iterator() { - if find_mnemonic_index_from_word(word, language).is_ok() { - return Ok(*language); - } - } - return Err(MnemonicError::UnknownLanguage); - }, - Ordering::Greater => { - for word in words { - let mut languages = Vec::with_capacity(MnemonicLanguage::iterator().len()); - // detect all languages in which a word falls into - for language in MnemonicLanguage::iterator() { - if find_mnemonic_index_from_word(word, language).is_ok() { - languages.push(*language); - } - } - // check if at least one of the languages is consistent for all other words against languages yielded - // from the initial word for this iteration - for language in languages { - let mut consistent = true; - for compare in words { - if compare != word && find_mnemonic_index_from_word(compare, &language).is_err() { - consistent = false; - } - } - if consistent { - return Ok(language); - } - } - } - }, - } - - Err(MnemonicError::UnknownLanguage) -} - /// Generates a vector of bytes that represent the provided mnemonic sequence of words, the language of the mnemonic /// sequence is detected pub fn to_bytes(mnemonic_seq: &[String]) -> Result, MnemonicError> { - let language = self::detect_language(mnemonic_seq)?; + let language = MnemonicLanguage::detect_language(mnemonic_seq)?; to_bytes_with_language(mnemonic_seq, &language) } @@ -336,7 +349,10 @@ mod test { "opera".to_string(), "abandon".to_string(), ]; - assert_eq!(detect_language(&words1), Ok(MnemonicLanguage::English)); + assert_eq!( + MnemonicLanguage::detect_language(&words1), + Ok(MnemonicLanguage::English) + ); // English/Spanish + English/French + Italian/Spanish let words2 = vec![ @@ -346,7 +362,7 @@ mod test { "abandon".to_string(), "tipico".to_string(), ]; - assert_eq!(detect_language(&words2).is_err(), true); + assert_eq!(MnemonicLanguage::detect_language(&words2).is_err(), true); // bounds check (last word is invalid) let words3 = vec![ @@ -356,16 +372,16 @@ mod test { "abandon".to_string(), "topazio".to_string(), ]; - assert_eq!(detect_language(&words3).is_err(), true); + assert_eq!(MnemonicLanguage::detect_language(&words3).is_err(), true); // building up a word list: English/French + French -> French let mut words = Vec::with_capacity(3); words.push("concert".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English)); words.push("abandon".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::English)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::English)); words.push("barbier".to_string()); - assert_eq!(detect_language(&words), Ok(MnemonicLanguage::French)); + assert_eq!(MnemonicLanguage::detect_language(&words), Ok(MnemonicLanguage::French)); } #[test] diff --git a/base_layer/wallet_ffi/src/enums.rs b/base_layer/wallet_ffi/src/enums.rs index 4d54dfc38e..bb1d90b598 100644 --- a/base_layer/wallet_ffi/src/enums.rs +++ b/base_layer/wallet_ffi/src/enums.rs @@ -27,4 +27,5 @@ pub enum SeedWordPushResult { SeedPhraseComplete, InvalidSeedPhrase, InvalidObject, + NoLanguageMatch, } diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index 2a65bf8fd5..566ec880d4 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -1020,11 +1020,13 @@ pub unsafe extern "C" fn seed_words_get_at( /// /// ## Returns /// 'c_uchar' - Returns a u8 version of the `SeedWordPushResult` enum indicating whether the word was not a valid seed -/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase +/// word, if the push was successful and whether the push was successful and completed the full Seed Phrase. +/// `seed_words` is only modified in the event of a `SuccessfulPush`. /// '0' -> InvalidSeedWord /// '1' -> SuccessfulPush /// '2' -> SeedPhraseComplete /// '3' -> InvalidSeedPhrase +/// '4' -> NoLanguageMatch, /// # Safety /// The ```string_destroy``` method must be called when finished with a string from rust to prevent a memory leak #[no_mangle] @@ -1082,28 +1084,64 @@ pub unsafe extern "C" fn seed_words_push_word( }, } - if MnemonicLanguage::from(word_string.as_str()).is_err() { - log::error!(target: LOG_TARGET, "{} is not a valid mnemonic seed word", word_string); - return SeedWordPushResult::InvalidSeedWord as u8; + // Seed words is currently empty, this is the first word + if (*seed_words).0.is_empty() { + (*seed_words).0.push(word_string); + return SeedWordPushResult::SuccessfulPush as u8; } - (*seed_words).0.push(word_string); - if (*seed_words).0.len() >= 24 { - return if let Err(e) = CipherSeed::from_mnemonic(&(*seed_words).0, None) { + // Try push to a temporary copy first to prevent existing object becoming invalid + let mut temp = (*seed_words).0.clone(); + + if let Ok(language) = MnemonicLanguage::detect_language(&temp) { + temp.push(word_string.clone()); + // Check words in temp are still consistent for a language, note that detected language can change + // depending on word added + if MnemonicLanguage::detect_language(&temp).is_ok() { + if temp.len() >= 24 { + if let Err(e) = CipherSeed::from_mnemonic(&temp, None) { + log::error!( + target: LOG_TARGET, + "Problem building valid private seed from seed phrase: {:?}", + e + ); + error = LibWalletError::from(WalletError::KeyManagerError(e)).code; + ptr::swap(error_out, &mut error as *mut c_int); + return SeedWordPushResult::InvalidSeedPhrase as u8; + }; + } + + (*seed_words).0.push(word_string); + + // Note: test for a validity was already done so we can just check length here + if (*seed_words).0.len() < 24 { + SeedWordPushResult::SuccessfulPush as u8 + } else { + SeedWordPushResult::SeedPhraseComplete as u8 + } + } else { log::error!( target: LOG_TARGET, - "Problem building valid private seed from seed phrase: {:?}", - e + "Words in seed phrase do not match any language after trying to add word: `{:?}`, previously words \ + were detected to be in: `{:?}`", + word_string, + language ); - error = LibWalletError::from(WalletError::KeyManagerError(e)).code; - ptr::swap(error_out, &mut error as *mut c_int); - SeedWordPushResult::InvalidSeedPhrase as u8 - } else { - SeedWordPushResult::SeedPhraseComplete as u8 - }; + SeedWordPushResult::NoLanguageMatch as u8 + } + } else { + // Seed words are invalid, shouldn't normally be reachable + log::error!( + target: LOG_TARGET, + "Words in seed phrase do not match any language prior to adding word: `{:?}`", + word_string + ); + let error_msg = "Invalid seed words object, no language can be detected."; + log::error!(target: LOG_TARGET, "{}", error_msg); + error = LibWalletError::from(InterfaceError::InvalidArgument(error_msg.to_string())).code; + ptr::swap(error_out, &mut error as *mut c_int); + SeedWordPushResult::InvalidObject as u8 } - - SeedWordPushResult::SuccessfulPush as u8 } /// Frees memory for a TariSeedWords