From 079fddbaa65c08bbd8595cb73e4eecc532d3f7a8 Mon Sep 17 00:00:00 2001 From: Justin Berman Date: Mon, 19 Feb 2024 19:03:02 -0800 Subject: [PATCH] monero: only mask user features on new polyseed, not on decode (#503) * monero: only mask user features on new polyseed, not on decode - This commit ensures a polyseed string that has unsupported features correctly errors on decode (rather than panic in debug build or return an incorrect successful response in prod build) - Also avoids panicking when checksum calculation is unexpectedly wrong Polyseed reference impl for feature masking: - polyseed_create: https://github.com/tevador/polyseed/blob/b7c35bb3c6b91e481ecb04fc235eaff69c507fa1/src/polyseed.c#L61 - polyseed_decode: https://github.com/tevador/polyseed/blob/b7c35bb3c6b91e481ecb04fc235eaff69c507fa1/src/polyseed.c#L212 * PR comments * Make from_internal a member of Polyseed * Add accidentally removed newline --------- Co-authored-by: Luke Parker --- coins/monero/src/tests/seed.rs | 12 ++++++- coins/monero/src/wallet/seed/polyseed.rs | 42 +++++++++++++++--------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/coins/monero/src/tests/seed.rs b/coins/monero/src/tests/seed.rs index 878293a0..ba6280df 100644 --- a/coins/monero/src/tests/seed.rs +++ b/coins/monero/src/tests/seed.rs @@ -7,7 +7,7 @@ use curve25519_dalek::scalar::Scalar; use crate::{ hash, wallet::seed::{ - Seed, SeedType, + Seed, SeedType, SeedError, classic::{self, trim_by_lang}, polyseed, }, @@ -469,3 +469,13 @@ fn test_polyseed() { } } } + +#[test] +fn test_invalid_polyseed() { + // This seed includes unsupported features bits and should error on decode + let seed = "include domain claim resemble urban hire lunch bird \ + crucial fire best wife ring warm ignore model" + .into(); + let res = Seed::from_string(Zeroizing::new(seed)); + assert_eq!(res, Err(SeedError::UnsupportedFeatures)); +} diff --git a/coins/monero/src/wallet/seed/polyseed.rs b/coins/monero/src/wallet/seed/polyseed.rs index 519ba7d4..cecdef9e 100644 --- a/coins/monero/src/wallet/seed/polyseed.rs +++ b/coins/monero/src/wallet/seed/polyseed.rs @@ -217,6 +217,31 @@ impl Polyseed { poly } + fn from_internal( + language: Language, + masked_features: u8, + encoded_birthday: u16, + entropy: Zeroizing<[u8; 32]>, + ) -> Result { + if !polyseed_features_supported(masked_features) { + Err(SeedError::UnsupportedFeatures)?; + } + + if !valid_entropy(&entropy) { + Err(SeedError::InvalidEntropy)?; + } + + let mut res = Polyseed { + language, + birthday: encoded_birthday, + features: masked_features, + entropy, + checksum: 0, + }; + res.checksum = poly_eval(&res.to_poly()); + Ok(res) + } + /// Create a new `Polyseed` with specific internals. /// /// `birthday` is defined in seconds since the Unix epoch. @@ -226,20 +251,7 @@ impl Polyseed { birthday: u64, entropy: Zeroizing<[u8; 32]>, ) -> Result { - let features = user_features(features); - if !polyseed_features_supported(features) { - Err(SeedError::UnsupportedFeatures)?; - } - - let birthday = birthday_encode(birthday); - - if !valid_entropy(&entropy) { - Err(SeedError::InvalidEntropy)?; - } - - let mut res = Polyseed { language, birthday, features, entropy, checksum: 0 }; - res.checksum = poly_eval(&res.to_poly()); - Ok(res) + Self::from_internal(language, user_features(features), birthday_encode(birthday), entropy) } /// Create a new `Polyseed`. @@ -370,7 +382,7 @@ impl Polyseed { let features = u8::try_from(extra >> DATE_BITS).expect("couldn't convert extra >> DATE_BITS to u8"); - let res = Polyseed::from(lang, features, birthday_decode(birthday), entropy); + let res = Self::from_internal(lang, features, birthday, entropy); if let Ok(res) = res.as_ref() { debug_assert_eq!(res.checksum, checksum); }