This repository was archived by the owner on Oct 22, 2026. It is now read-only.
Repository navigation
descriptor: reject keys deeper than BIP32 allows - #1065
Merged
Merged
Conversation
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 this error into a panic labeled "cryptographically unreachable". A descriptor string can trigger it: an xpub with a long enough path parses, becomes definite, and panics when its script pubkey is computed. Return the new NonDefiniteKeyError::MaximumDepthExceeded from DefiniteDescriptorKey::new instead. Derivation at an index and into_definite go through it, so they return the error too. Only the steps after the extended key count, since its depth already includes its origin steps. Assisted-by: Claude Opus 5.5
A key expression that derives past BIP32's maximum depth of 255 can never produce a key. The previous commit rejects such a key when it becomes definite. Reject it when parsing too, so that the error points at the string that caused it. Count the steps after the extended key, plus one for a wildcard, for xpubs and xprvs, including multipath keys. Origin steps don't count. Assisted-by: Claude Opus 5.5
This was referenced Oct 12, 2026
Member
|
Can you backport to the |
Contributor
Author
Done: #1074 Some changes were made and one more commit was added from another PR (see the details in the PR description). |
apoelstra
added a commit
that referenced
this pull request
Oct 17, 2026
561042b descriptor: reject keys deeper than BIP32 allows when parsing (Boris Nagaev) 6d3aa6e descriptor: reject definite keys deeper than BIP32 allows (Boris Nagaev) 694525f Fix hardened derivation path error handling in `at_derivation_index` (Velnbur) Pull request description: Backport of #1065. It also needs 97c948d from #913, which isn't on 13.x: without it, `at_derivation_index` panics on the new depth error instead of returning it. 13.x has no `NonDefiniteKeyError::NoWildcard`, `into_definite` or `derive_at_index`, so those parts of #1065 are left out, and its test uses `at_derivation_index(0)` instead of `into_definite()`. 🤖 AI disclosure: the backport was prepared with Claude Opus 5.5. ACKs for top commit: apoelstra: ACK 561042b; successfully ran local tests Tree-SHA512: 06c9574f642cb7b85d319ae7ce705dc267c1d44eb2286b213640885c3dea05a12b294b27df094c9d07630afd3ab3d24fe103802245fd84ec454d9972cad05c2f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DefiniteDescriptorKey::newchecks 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 returnMaximumDepthExceeded, andderive_public_keyturns 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, passesinto_definite(), and panics inscript_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.6made it a recoverable error (rust-bitcoin/rust-bitcoin#4434), which miniscript doesn't handle.The first commit makes
DefiniteDescriptorKey::newreturn the newNonDefiniteKeyError::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.