diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index 78eaf93be..dda3f96da 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -439,6 +439,7 @@ pub enum NonDefiniteKeyError { Multipath, HardenedStep, NoWildcard, + MaximumDepthExceeded, } impl fmt::Display for NonDefiniteKeyError { @@ -452,6 +453,9 @@ impl fmt::Display for NonDefiniteKeyError { Self::NoWildcard => { f.write_str("descriptor does not have a wildcard; cannot derive at index") } + Self::MaximumDepthExceeded => f.write_str( + "key deeper than BIP32's maximum depth of 255 cannot be a DerivedDescriptorKey", + ), } } } @@ -473,6 +477,7 @@ pub enum MalformedKeyDataKind { InvalidPublicKeyLength, InvalidWildcardInDerivationPath, KeyTooShort, + MaximumDepthExceeded, MultipleFingerprintsInPublicKey, MultipleDerivationPathIndexSteps, NoKeyAfterOrigin, @@ -492,6 +497,9 @@ impl fmt::Display for MalformedKeyDataKind { Self::InvalidPublicKeyLength => "public keys must be 64, 66 or 130 characters in size", Self::InvalidWildcardInDerivationPath => "'*' may only appear as last element in a derivation path", Self::KeyTooShort => "key too short", + Self::MaximumDepthExceeded => { + "key derivation would go deeper than BIP32's maximum depth of 255" + } Self::MultipleFingerprintsInPublicKey => "multiple ']' in Descriptor Public Key", Self::MultipleDerivationPathIndexSteps => "'<' may only appear once in a derivation path", Self::NoKeyAfterOrigin => "no key after origin", @@ -744,7 +752,13 @@ impl FromStr for DescriptorPublicKey { match key_part.get(..4) { Some("xprv" | "tprv") => Err(DescriptorKeyParseError::UnexpectedXPrivateKey), Some("xpub" | "tpub") => { - let (xpub, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?; + let (xpub, derivation_paths, wildcard) = + parse_xkey_deriv::(key_part)?; + if derivation_exceeds_max_depth(xpub.depth, &derivation_paths, wildcard) { + return Err(DescriptorKeyParseError::MalformedKeyData( + MalformedKeyDataKind::MaximumDepthExceeded, + )); + } if derivation_paths.len() > 1 { Ok(Self::MultiXPub(DescriptorMultiXKey { origin, @@ -964,6 +978,24 @@ impl DescriptorPublicKey { false } + /// Whether deriving this key, including a wildcard step, would go deeper than BIP32's maximum + /// depth of 255. + fn exceeds_max_depth(&self) -> bool { + match self { + Self::Single(..) => false, + Self::XPub(xpub) => derivation_exceeds_max_depth( + xpub.xkey.depth, + core::slice::from_ref(&xpub.derivation_path), + xpub.wildcard, + ), + Self::MultiXPub(xpub) => derivation_exceeds_max_depth( + xpub.xkey.depth, + xpub.derivation_paths.paths(), + xpub.wildcard, + ), + } + } + /// Replaces any wildcard (i.e. `/*`) in the key with a particular derivation index, turning it into a /// *definite* key (i.e. one where all the derivation paths are set). /// @@ -978,6 +1010,7 @@ impl DescriptorPublicKey { /// - If `index` is hardened. /// - If derivation path contains a hardened step. /// - If the key contains multi-path derivations + /// - If deriving the key would go deeper than BIP32's maximum depth of 255 pub fn at_derivation_index( self, index: u32, @@ -1068,7 +1101,13 @@ impl FromStr for DescriptorSecretKey { .map_err(DescriptorKeyParseError::WifPrivateKey)?; Ok(Self::Single(SinglePriv { key: sk, origin })) } else { - let (xpriv, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?; + let (xpriv, derivation_paths, wildcard) = + parse_xkey_deriv::(key_part)?; + if derivation_exceeds_max_depth(xpriv.depth, &derivation_paths, wildcard) { + return Err(DescriptorKeyParseError::MalformedKeyData( + MalformedKeyDataKind::MaximumDepthExceeded, + )); + } if derivation_paths.len() > 1 { Ok(Self::MultiXPrv(DescriptorMultiXKey { origin, @@ -1151,6 +1190,20 @@ fn parse_key_origin(s: &str) -> Result<(&str, Option), Descrip } } +/// Whether deriving from an extended key at `depth` along any of `paths`, plus one more step for a +/// wildcard, would go deeper than BIP32's maximum depth of 255. +/// +/// The paths hold only the steps after the extended key. Its depth already includes the steps of +/// its origin, so those are not counted again. +fn derivation_exceeds_max_depth( + depth: u8, + paths: &[bip32::DerivationPath], + wildcard: Wildcard, +) -> bool { + let longest = paths.iter().map(|path| path.len()).max().unwrap_or(0); + longest + usize::from(wildcard != Wildcard::None) > usize::from(u8::MAX - depth) +} + pub(crate) fn parse_xkey_deriv( key_deriv: &str, ) -> Result<(Key, Vec, Wildcard), DescriptorKeyParseError> @@ -1411,9 +1464,10 @@ impl DefiniteDescriptorKey { } } - /// Construct an instance from a descriptor key and a derivation index + /// Constructs an instance from a descriptor key. /// - /// Returns `None` if the key contains a wildcard + /// Returns an error if the key has a wildcard, a hardened derivation step or several derivation + /// paths, or if deriving it would go deeper than BIP32's maximum depth of 255. pub fn new(key: DescriptorPublicKey) -> Result { if key.has_wildcard() { Err(NonDefiniteKeyError::Wildcard) @@ -1421,6 +1475,8 @@ impl DefiniteDescriptorKey { Err(NonDefiniteKeyError::HardenedStep) } else if key.is_multipath() { Err(NonDefiniteKeyError::Multipath) + } else if key.exceeds_max_depth() { + Err(NonDefiniteKeyError::MaximumDepthExceeded) } else { Ok(Self(key)) } @@ -1527,11 +1583,11 @@ mod test { use super::{ DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, - MiniscriptKey, Wildcard, + DescriptorXKey, MalformedKeyDataKind, MiniscriptKey, Wildcard, }; use crate::descriptor::key::NonDefiniteKeyError; use crate::prelude::*; - use crate::DefiniteDescriptorKey; + use crate::{DefiniteDescriptorKey, Descriptor}; #[test] fn parse_descriptor_key_errors() { @@ -1946,6 +2002,90 @@ mod test { )); } + #[test] + fn definite_key_maximum_depth() { + let key = |depth: u8, steps: usize, wildcard: Wildcard| { + let mut xkey = bip32::Xpub::from_str("xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8").unwrap(); + xkey.depth = depth; + DescriptorPublicKey::XPub(DescriptorXKey { + origin: None, + xkey, + derivation_path: vec![bip32::ChildNumber::from(0); steps].into(), + wildcard, + }) + }; + let new = |key| DefiniteDescriptorKey::new(key).map(|_| ()); + let too_deep = Err(NonDefiniteKeyError::MaximumDepthExceeded); + + // BIP32 depth stops at 255, so an xpub at `depth` can take 255 - depth more steps. + assert_eq!(new(key(0, 255, Wildcard::None)), Ok(())); + assert_eq!(new(key(0, 256, Wildcard::None)), too_deep); + assert_eq!(new(key(200, 55, Wildcard::None)), Ok(())); + assert_eq!(new(key(200, 56, Wildcard::None)), too_deep); + + // Deriving at an index turns the wildcard into one more step. + assert!(key(200, 54, Wildcard::Unhardened) + .at_derivation_index(7) + .is_ok()); + assert_eq!( + key(200, 55, Wildcard::Unhardened) + .at_derivation_index(7) + .map(|_| ()), + too_deep + ); + + // Origin steps don't count, since the xpub's depth already includes them. + let mut key_with_origin = key(0, 255, Wildcard::None); + if let DescriptorPublicKey::XPub(ref mut xpub) = key_with_origin { + xpub.origin = Some(( + bip32::Fingerprint::default(), + vec![bip32::ChildNumber::from(0); 300].into(), + )); + } + assert_eq!(new(key_with_origin), Ok(())); + + // A descriptor with such a key doesn't become definite, so it can't panic later when + // deriving its script pubkey. + let desc = Descriptor::new_tr(key(0, 256, Wildcard::None), None).unwrap(); + assert_eq!(desc.into_definite().map(|_| ()), too_deep); + } + + #[test] + fn parse_key_maximum_depth() { + // BIP32 test vector 1 master keys, at depth 0. + let xpub = "xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8"; + let xprv = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"; + let steps = |n: usize| "/0".repeat(n); + let public = |s: String| DescriptorPublicKey::from_str(&s).map(|_| ()); + let secret = |s: String| DescriptorSecretKey::from_str(&s).map(|_| ()); + let too_deep = Err(DescriptorKeyParseError::MalformedKeyData( + MalformedKeyDataKind::MaximumDepthExceeded, + )); + + // BIP32 depth stops at 255. A wildcard or a multipath step counts as one more step. + assert_eq!(public(format!("{xpub}{}", steps(255))), Ok(())); + assert_eq!(public(format!("{xpub}{}", steps(256))), too_deep); + assert_eq!(public(format!("{xpub}{}/*", steps(254))), Ok(())); + assert_eq!(public(format!("{xpub}{}/*", steps(255))), too_deep); + assert_eq!(public(format!("{xpub}{}/<0;1>", steps(254))), Ok(())); + assert_eq!(public(format!("{xpub}{}/<0;1>", steps(255))), too_deep); + assert_eq!(secret(format!("{xprv}{}/*", steps(254))), Ok(())); + assert_eq!(secret(format!("{xprv}{}/*", steps(255))), too_deep); + + // An xpub at depth 200 can take 55 more steps. + let mut xkey = bip32::Xpub::from_str(xpub).unwrap(); + xkey.depth = 200; + assert_eq!(public(format!("{xkey}{}", steps(55))), Ok(())); + assert_eq!(public(format!("{xkey}{}", steps(56))), too_deep); + + // Origin steps don't count, since the xpub's depth already includes them. + assert_eq!(public(format!("[d34db33f{}]{xpub}{}", steps(300), steps(255))), Ok(())); + + // A descriptor with such a key fails to parse, before anything can be derived from it. + let desc = format!("tr({xpub}{})", steps(256)); + assert!(Descriptor::::from_str(&desc).is_err()); + } + #[test] fn test_xkey_network() { use bitcoin::NetworkKind; diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 2747857b1..55a7f7075 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -697,6 +697,7 @@ impl Descriptor { /// # Errors /// - If the descriptor contains wildcards /// - If the descriptor contains multi-path derivations + /// - If deriving a key would go deeper than BIP32's maximum depth of 255 pub fn into_definite(&self) -> Result, NonDefiniteKeyError> { if self.has_wildcard() { return Err(NonDefiniteKeyError::Wildcard); @@ -725,6 +726,7 @@ impl Descriptor { /// - If the descriptor contains no wildcards /// - If index >= 2^31 /// - If the descriptor contains multi-path derivations + /// - If deriving a key would go deeper than BIP32's maximum depth of 255 #[allow(deprecated)] pub fn derive_at_index(&self, index: u32) -> DerivationResult { if !self.has_wildcard() { @@ -794,8 +796,8 @@ impl Descriptor { /// /// # Errors /// - /// This function will return an error for multi-path descriptors - /// or if hardened derivation is attempted, + /// This function will return an error for multi-path descriptors, if hardened derivation is + /// attempted, or if deriving a key would go deeper than BIP32's maximum depth of 255. #[allow(deprecated)] pub fn derived_descriptor( &self,