Skip to content
This repository was archived by the owner on Oct 22, 2026. It is now read-only.

Commit 96a1c91

Browse files
committed
Merge #1065: descriptor: reject keys deeper than BIP32 allows
f2862f7 descriptor: reject keys deeper than BIP32 allows when parsing (Boris Nagaev) 613ed0d descriptor: reject definite keys deeper than BIP32 allows (Boris Nagaev) Pull request description: `DefiniteDescriptorKey::new` checks for wildcards, hardened steps and multipath derivation, but not for BIP32's maximum depth of 255. A key that passes these checks but derives past that depth makes rust-bitcoin return `MaximumDepthExceeded`, and `derive_public_key` turns that error into a panic labeled "cryptographically unreachable". A descriptor string is enough to trigger it: `tr(xpub/0/0/.../0)` with 256 steps below a master xpub parses, passes `into_definite()`, and panics in `script_pubkey()`. This is missing validation in miniscript. Deriving that deep used to overflow rust-bitcoin's depth counter (rust-bitcoin/rust-bitcoin#4308). `rust-bitcoin 0.32.6` made it a recoverable error (rust-bitcoin/rust-bitcoin#4434), which miniscript doesn't handle. The first commit makes `DefiniteDescriptorKey::new` return the new `NonDefiniteKeyError::MaximumDepthExceeded`. `into_definite()` and derivation at an index go through it, so they report the error too. Only the steps after the extended key count, since its depth already includes the steps of its origin. The second commit rejects such key expressions when parsing, with the new `MalformedKeyDataKind::MaximumDepthExceeded`. It counts a wildcard as one more step and covers xprvs and multipath keys. It's a separate commit because some strings that parse today will stop parsing, even though no key can be derived from them. It can be dropped if parsing should stay unchanged. Together, they remove the deterministic depth panic. Derivation can still fail in BIP32's invalid-child case, which has a probability below 2^-127 and stays `unreachable!`. Tests cover depths 255 and 256, a nonzero starting depth, wildcard derivation, multipath keys, xprvs and origin paths. This pre-existing bug was found while working on #954. 🤖 _AI disclosure: The patch, tests and this description were generated with Claude Opus 5.5. Codex reviewed the approach._ ACKs for top commit: apoelstra: ACK f2862f7; successfully ran local tests Tree-SHA512: 671b1a6b284fe1a3357c9b39236ad41cbe267059c18c4db93a147c76038fb349db7897a6662530d958ca61e97c11eb81718c14316dd6f8df2ad1449612bf5dd4
2 parents d7d1610 + f2862f7 commit 96a1c91

2 files changed

Lines changed: 150 additions & 8 deletions

File tree

‎src/descriptor/key.rs‎

Lines changed: 146 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ pub enum NonDefiniteKeyError {
439439
Multipath,
440440
HardenedStep,
441441
NoWildcard,
442+
MaximumDepthExceeded,
442443
}
443444

444445
impl fmt::Display for NonDefiniteKeyError {
@@ -452,6 +453,9 @@ impl fmt::Display for NonDefiniteKeyError {
452453
Self::NoWildcard => {
453454
f.write_str("descriptor does not have a wildcard; cannot derive at index")
454455
}
456+
Self::MaximumDepthExceeded => f.write_str(
457+
"key deeper than BIP32's maximum depth of 255 cannot be a DerivedDescriptorKey",
458+
),
455459
}
456460
}
457461
}
@@ -473,6 +477,7 @@ pub enum MalformedKeyDataKind {
473477
InvalidPublicKeyLength,
474478
InvalidWildcardInDerivationPath,
475479
KeyTooShort,
480+
MaximumDepthExceeded,
476481
MultipleFingerprintsInPublicKey,
477482
MultipleDerivationPathIndexSteps,
478483
NoKeyAfterOrigin,
@@ -492,6 +497,9 @@ impl fmt::Display for MalformedKeyDataKind {
492497
Self::InvalidPublicKeyLength => "public keys must be 64, 66 or 130 characters in size",
493498
Self::InvalidWildcardInDerivationPath => "'*' may only appear as last element in a derivation path",
494499
Self::KeyTooShort => "key too short",
500+
Self::MaximumDepthExceeded => {
501+
"key derivation would go deeper than BIP32's maximum depth of 255"
502+
}
495503
Self::MultipleFingerprintsInPublicKey => "multiple ']' in Descriptor Public Key",
496504
Self::MultipleDerivationPathIndexSteps => "'<' may only appear once in a derivation path",
497505
Self::NoKeyAfterOrigin => "no key after origin",
@@ -744,7 +752,13 @@ impl FromStr for DescriptorPublicKey {
744752
match key_part.get(..4) {
745753
Some("xprv" | "tprv") => Err(DescriptorKeyParseError::UnexpectedXPrivateKey),
746754
Some("xpub" | "tpub") => {
747-
let (xpub, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?;
755+
let (xpub, derivation_paths, wildcard) =
756+
parse_xkey_deriv::<bip32::Xpub, _>(key_part)?;
757+
if derivation_exceeds_max_depth(xpub.depth, &derivation_paths, wildcard) {
758+
return Err(DescriptorKeyParseError::MalformedKeyData(
759+
MalformedKeyDataKind::MaximumDepthExceeded,
760+
));
761+
}
748762
if derivation_paths.len() > 1 {
749763
Ok(Self::MultiXPub(DescriptorMultiXKey {
750764
origin,
@@ -971,6 +985,24 @@ impl DescriptorPublicKey {
971985
false
972986
}
973987

988+
/// Whether deriving this key, including a wildcard step, would go deeper than BIP32's maximum
989+
/// depth of 255.
990+
fn exceeds_max_depth(&self) -> bool {
991+
match self {
992+
Self::Single(..) => false,
993+
Self::XPub(xpub) => derivation_exceeds_max_depth(
994+
xpub.xkey.depth,
995+
core::slice::from_ref(&xpub.derivation_path),
996+
xpub.wildcard,
997+
),
998+
Self::MultiXPub(xpub) => derivation_exceeds_max_depth(
999+
xpub.xkey.depth,
1000+
xpub.derivation_paths.paths(),
1001+
xpub.wildcard,
1002+
),
1003+
}
1004+
}
1005+
9741006
/// Replaces any wildcard (i.e. `/*`) in the key with a particular derivation index, turning it into a
9751007
/// *definite* key (i.e. one where all the derivation paths are set).
9761008
///
@@ -985,6 +1017,7 @@ impl DescriptorPublicKey {
9851017
/// - If `index` is hardened.
9861018
/// - If derivation path contains a hardened step.
9871019
/// - If the key contains multi-path derivations
1020+
/// - If deriving the key would go deeper than BIP32's maximum depth of 255
9881021
pub fn at_derivation_index(
9891022
self,
9901023
index: u32,
@@ -1075,7 +1108,13 @@ impl FromStr for DescriptorSecretKey {
10751108
.map_err(DescriptorKeyParseError::WifPrivateKey)?;
10761109
Ok(Self::Single(SinglePriv { key: sk, origin }))
10771110
} else {
1078-
let (xpriv, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?;
1111+
let (xpriv, derivation_paths, wildcard) =
1112+
parse_xkey_deriv::<bip32::Xpriv, _>(key_part)?;
1113+
if derivation_exceeds_max_depth(xpriv.depth, &derivation_paths, wildcard) {
1114+
return Err(DescriptorKeyParseError::MalformedKeyData(
1115+
MalformedKeyDataKind::MaximumDepthExceeded,
1116+
));
1117+
}
10791118
if derivation_paths.len() > 1 {
10801119
Ok(Self::MultiXPrv(DescriptorMultiXKey {
10811120
origin,
@@ -1158,6 +1197,20 @@ fn parse_key_origin(s: &str) -> Result<(&str, Option<bip32::KeySource>), Descrip
11581197
}
11591198
}
11601199

1200+
/// Whether deriving from an extended key at `depth` along any of `paths`, plus one more step for a
1201+
/// wildcard, would go deeper than BIP32's maximum depth of 255.
1202+
///
1203+
/// The paths hold only the steps after the extended key. Its depth already includes the steps of
1204+
/// its origin, so those are not counted again.
1205+
fn derivation_exceeds_max_depth(
1206+
depth: u8,
1207+
paths: &[bip32::DerivationPath],
1208+
wildcard: Wildcard,
1209+
) -> bool {
1210+
let longest = paths.iter().map(|path| path.len()).max().unwrap_or(0);
1211+
longest + usize::from(wildcard != Wildcard::None) > usize::from(u8::MAX - depth)
1212+
}
1213+
11611214
pub(crate) fn parse_xkey_deriv<Key, E>(
11621215
key_deriv: &str,
11631216
) -> Result<(Key, Vec<bip32::DerivationPath>, Wildcard), DescriptorKeyParseError>
@@ -1418,16 +1471,19 @@ impl DefiniteDescriptorKey {
14181471
}
14191472
}
14201473

1421-
/// Construct an instance from a descriptor key and a derivation index
1474+
/// Constructs an instance from a descriptor key.
14221475
///
1423-
/// Returns `None` if the key contains a wildcard
1476+
/// Returns an error if the key has a wildcard, a hardened derivation step or several derivation
1477+
/// paths, or if deriving it would go deeper than BIP32's maximum depth of 255.
14241478
pub fn new(key: DescriptorPublicKey) -> Result<Self, NonDefiniteKeyError> {
14251479
if key.has_wildcard() {
14261480
Err(NonDefiniteKeyError::Wildcard)
14271481
} else if key.has_hardened_step() {
14281482
Err(NonDefiniteKeyError::HardenedStep)
14291483
} else if key.is_multipath() {
14301484
Err(NonDefiniteKeyError::Multipath)
1485+
} else if key.exceeds_max_depth() {
1486+
Err(NonDefiniteKeyError::MaximumDepthExceeded)
14311487
} else {
14321488
Ok(Self(key))
14331489
}
@@ -1537,11 +1593,11 @@ mod test {
15371593

15381594
use super::{
15391595
DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey,
1540-
MiniscriptKey, Wildcard,
1596+
DescriptorXKey, MalformedKeyDataKind, MiniscriptKey, Wildcard,
15411597
};
15421598
use crate::descriptor::key::NonDefiniteKeyError;
15431599
use crate::prelude::*;
1544-
use crate::DefiniteDescriptorKey;
1600+
use crate::{DefiniteDescriptorKey, Descriptor};
15451601

15461602
#[test]
15471603
fn parse_descriptor_key_errors() {
@@ -1964,6 +2020,90 @@ mod test {
19642020
));
19652021
}
19662022

2023+
#[test]
2024+
fn definite_key_maximum_depth() {
2025+
let key = |depth: u8, steps: usize, wildcard: Wildcard| {
2026+
let mut xkey = bip32::Xpub::from_str("xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8").unwrap();
2027+
xkey.depth = depth;
2028+
DescriptorPublicKey::XPub(DescriptorXKey {
2029+
origin: None,
2030+
xkey,
2031+
derivation_path: vec![bip32::ChildNumber::from(0); steps].into(),
2032+
wildcard,
2033+
})
2034+
};
2035+
let new = |key| DefiniteDescriptorKey::new(key).map(|_| ());
2036+
let too_deep = Err(NonDefiniteKeyError::MaximumDepthExceeded);
2037+
2038+
// BIP32 depth stops at 255, so an xpub at `depth` can take 255 - depth more steps.
2039+
assert_eq!(new(key(0, 255, Wildcard::None)), Ok(()));
2040+
assert_eq!(new(key(0, 256, Wildcard::None)), too_deep);
2041+
assert_eq!(new(key(200, 55, Wildcard::None)), Ok(()));
2042+
assert_eq!(new(key(200, 56, Wildcard::None)), too_deep);
2043+
2044+
// Deriving at an index turns the wildcard into one more step.
2045+
assert!(key(200, 54, Wildcard::Unhardened)
2046+
.at_derivation_index(7)
2047+
.is_ok());
2048+
assert_eq!(
2049+
key(200, 55, Wildcard::Unhardened)
2050+
.at_derivation_index(7)
2051+
.map(|_| ()),
2052+
too_deep
2053+
);
2054+
2055+
// Origin steps don't count, since the xpub's depth already includes them.
2056+
let mut key_with_origin = key(0, 255, Wildcard::None);
2057+
if let DescriptorPublicKey::XPub(ref mut xpub) = key_with_origin {
2058+
xpub.origin = Some((
2059+
bip32::Fingerprint::default(),
2060+
vec![bip32::ChildNumber::from(0); 300].into(),
2061+
));
2062+
}
2063+
assert_eq!(new(key_with_origin), Ok(()));
2064+
2065+
// A descriptor with such a key doesn't become definite, so it can't panic later when
2066+
// deriving its script pubkey.
2067+
let desc = Descriptor::new_tr(key(0, 256, Wildcard::None), None).unwrap();
2068+
assert_eq!(desc.into_definite().map(|_| ()), too_deep);
2069+
}
2070+
2071+
#[test]
2072+
fn parse_key_maximum_depth() {
2073+
// BIP32 test vector 1 master keys, at depth 0.
2074+
let xpub = "xpub661MyMwAqRbcFtXgS5sYJABqqG9YLmC4Q1Rdap9gSE8NqtwybGhePY2gZ29ESFjqJoCu1Rupje8YtGqsefD265TMg7usUDFdp6W1EGMcet8";
2075+
let xprv = "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi";
2076+
let steps = |n: usize| "/0".repeat(n);
2077+
let public = |s: String| DescriptorPublicKey::from_str(&s).map(|_| ());
2078+
let secret = |s: String| DescriptorSecretKey::from_str(&s).map(|_| ());
2079+
let too_deep = Err(DescriptorKeyParseError::MalformedKeyData(
2080+
MalformedKeyDataKind::MaximumDepthExceeded,
2081+
));
2082+
2083+
// BIP32 depth stops at 255. A wildcard or a multipath step counts as one more step.
2084+
assert_eq!(public(format!("{xpub}{}", steps(255))), Ok(()));
2085+
assert_eq!(public(format!("{xpub}{}", steps(256))), too_deep);
2086+
assert_eq!(public(format!("{xpub}{}/*", steps(254))), Ok(()));
2087+
assert_eq!(public(format!("{xpub}{}/*", steps(255))), too_deep);
2088+
assert_eq!(public(format!("{xpub}{}/<0;1>", steps(254))), Ok(()));
2089+
assert_eq!(public(format!("{xpub}{}/<0;1>", steps(255))), too_deep);
2090+
assert_eq!(secret(format!("{xprv}{}/*", steps(254))), Ok(()));
2091+
assert_eq!(secret(format!("{xprv}{}/*", steps(255))), too_deep);
2092+
2093+
// An xpub at depth 200 can take 55 more steps.
2094+
let mut xkey = bip32::Xpub::from_str(xpub).unwrap();
2095+
xkey.depth = 200;
2096+
assert_eq!(public(format!("{xkey}{}", steps(55))), Ok(()));
2097+
assert_eq!(public(format!("{xkey}{}", steps(56))), too_deep);
2098+
2099+
// Origin steps don't count, since the xpub's depth already includes them.
2100+
assert_eq!(public(format!("[d34db33f{}]{xpub}{}", steps(300), steps(255))), Ok(()));
2101+
2102+
// A descriptor with such a key fails to parse, before anything can be derived from it.
2103+
let desc = format!("tr({xpub}{})", steps(256));
2104+
assert!(Descriptor::<DescriptorPublicKey>::from_str(&desc).is_err());
2105+
}
2106+
19672107
#[test]
19682108
fn test_xkey_network() {
19692109
use bitcoin::NetworkKind;

‎src/descriptor/mod.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,7 @@ impl Descriptor<DescriptorPublicKey> {
699699
/// # Errors
700700
/// - If the descriptor contains wildcards
701701
/// - If the descriptor contains multi-path derivations
702+
/// - If deriving a key would go deeper than BIP32's maximum depth of 255
702703
pub fn into_definite(&self) -> Result<Descriptor<DefiniteDescriptorKey>, NonDefiniteKeyError> {
703704
if self.has_wildcard() {
704705
return Err(NonDefiniteKeyError::Wildcard);
@@ -727,6 +728,7 @@ impl Descriptor<DescriptorPublicKey> {
727728
/// - If the descriptor contains no wildcards
728729
/// - If index >= 2^31
729730
/// - If the descriptor contains multi-path derivations
731+
/// - If deriving a key would go deeper than BIP32's maximum depth of 255
730732
#[allow(deprecated)]
731733
pub fn derive_at_index(&self, index: u32) -> DerivationResult {
732734
if !self.has_wildcard() {
@@ -796,8 +798,8 @@ impl Descriptor<DescriptorPublicKey> {
796798
///
797799
/// # Errors
798800
///
799-
/// This function will return an error for multi-path descriptors
800-
/// or if hardened derivation is attempted,
801+
/// This function will return an error for multi-path descriptors, if hardened derivation is
802+
/// attempted, or if deriving a key would go deeper than BIP32's maximum depth of 255.
801803
#[allow(deprecated)]
802804
pub fn derived_descriptor<C: secp256k1::Verification>(
803805
&self,

0 commit comments

Comments
 (0)