diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs index f673ef698..ae22028bd 100644 --- a/src/descriptor/key.rs +++ b/src/descriptor/key.rs @@ -361,6 +361,7 @@ pub enum NonDefiniteKeyError { Wildcard, Multipath, HardenedStep, + MaximumDepthExceeded, } impl fmt::Display for NonDefiniteKeyError { @@ -371,6 +372,9 @@ impl fmt::Display for NonDefiniteKeyError { Self::HardenedStep => { f.write_str("key with hardened derivation steps cannot be a DerivedDescriptorKey") } + Self::MaximumDepthExceeded => f.write_str( + "key deeper than BIP32's maximum depth of 255 cannot be a DerivedDescriptorKey", + ), } } } @@ -392,6 +396,7 @@ pub enum MalformedKeyDataKind { InvalidPublicKeyLength, InvalidWildcardInDerivationPath, KeyTooShort, + MaximumDepthExceeded, MultipleFingerprintsInPublicKey, MultipleDerivationPathIndexSteps, NoKeyAfterOrigin, @@ -411,6 +416,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", @@ -677,7 +685,12 @@ impl FromStr for DescriptorPublicKey { let (key_part, origin) = parse_key_origin(s)?; if key_part.contains("pub") { - 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(DescriptorPublicKey::MultiXPub(DescriptorMultiXKey { origin, @@ -897,6 +910,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). /// @@ -909,7 +940,9 @@ impl DescriptorPublicKey { /// # Errors /// /// - 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, @@ -940,8 +973,7 @@ impl DescriptorPublicKey { DescriptorPublicKey::MultiXPub(_) => return Err(NonDefiniteKeyError::Multipath), }; - Ok(DefiniteDescriptorKey::new(definite) - .expect("The key should not contain any wildcards at this point")) + DefiniteDescriptorKey::new(definite) } /// Whether or not this key has multiple derivation paths. @@ -1001,7 +1033,13 @@ impl FromStr for DescriptorSecretKey { .map_err(DescriptorKeyParseError::WifPrivateKey)?; Ok(DescriptorSecretKey::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(DescriptorSecretKey::MultiXPrv(DescriptorMultiXKey { origin, @@ -1084,6 +1122,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> @@ -1346,9 +1398,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) @@ -1356,6 +1409,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)) } @@ -1461,11 +1516,12 @@ mod test { use serde_test::{assert_tokens, Token}; use super::{ - DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, MiniscriptKey, Wildcard, + DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, + 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() { @@ -1873,6 +1929,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.at_derivation_index(0).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 88244d403..750c7ec1f 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -660,6 +660,7 @@ impl Descriptor { /// # Errors /// - 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 pub fn at_derivation_index( &self, index: u32, @@ -705,8 +706,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. pub fn derived_descriptor( &self, secp: &secp256k1::Secp256k1,