descriptor: reject keys deeper than BIP32 allows

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::<bip32::Xpub, _>(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::<bip32::Xpriv, _>(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<bip32::KeySource>), 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, E>(
     key_deriv: &str,
 ) -> Result<(Key, Vec<bip32::DerivationPath>, Wildcard), DescriptorKeyParseError>
@@ -1411,16 +1464,19 @@ 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<Self, NonDefiniteKeyError> {
         if key.has_wildcard() {
             Err(NonDefiniteKeyError::Wildcard)
         } else if key.has_hardened_step() {
             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::<DescriptorPublicKey>::from_str(&desc).is_err());
+    }
+
     #[test]
     fn test_xkey_network() {
         use bitcoin::NetworkKind;

src/descriptor/mod.rs

@@ -697,6 +697,7 @@ impl Descriptor<DescriptorPublicKey> {
     /// # 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<Descriptor<DefiniteDescriptorKey>, NonDefiniteKeyError> {
         if self.has_wildcard() {
             return Err(NonDefiniteKeyError::Wildcard);
@@ -725,6 +726,7 @@ impl Descriptor<DescriptorPublicKey> {
     /// - 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<DescriptorPublicKey> {
     ///
     /// # 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<C: secp256k1::Verification>(
         &self,