What the developer wrote
Authored by benma's agent
btc: limit Miniscript encoding depth
Reject WSH policies and Taproot leaves that exceed the firmware-specific recursive encoding limit.
Cover the boundary and known deep-policy shape with tests.
What changed, and why it matters
This commit adds a safety limit to how deeply nested a Bitcoin policy (a set of spending rules) can be before the BitBox02 hardware wallet will reject it. Without such a limit, an attacker could craft an unusually deep policy that causes the wallet's stack memory to overflow during normal processing, potentially crashing the device or causing undefined behavior. The fix caps the allowed depth and adds tests to confirm both the boundary and a known problematic shape are blocked.
Treat this as a security hardening fix and include it in the next firmware release. Review whether other recursive Miniscript operations (e.g., satisfaction, script generation) need similar depth limits, and consider whether 64 is appropriate for the device's actual stack size under worst-case call frames.
Security signals we found
Adds explicit depth bound to prevent recursive stack exhaustion
Applies to both WSH and Taproot (TR) policy parsing paths
Includes boundary and regression-style tests for deep policies
Returns generic InvalidInput error, consistent with existing parse failures
Comment notes limit is conservative rather than exact stack threshold
Evidence from the diff
The patch introduces MAX_MINISCRIPT_ENCODE_DEPTH (64) and a validate_miniscript_depth() helper that rejects Miniscript expressions whose tree_height plus one (the root encoding frame) reaches the limit. It applies this check to WSH (Segwit v0) descriptors and to every Taproot leaf in a TR descriptor during parse(). Unit tests verify that exactly depth 64 is accepted, depth 65 is rejected, and a previously known deep policy shape is also rejected. The change is defensive: it prevents stack exhaustion from recursive Miniscript encoding of adversarially deep policies.
Changed components
src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rsBitcoin WSH descriptor parsingBitcoin Taproot (TR) descriptor parsingMiniscript encoding/validation pathInspect captured patch +109 / −0
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
index f46e37d..c66d4e7 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
@@ -27,6 +27,24 @@ use sha2::{Digest, Sha256};
// Arbitrary limit of keys that can be present in a policy.
const MAX_KEYS: usize = 20;
+// Conservative rather than exact stack threshold. Miniscript encoding uses relatively small
+// synchronous recursion frames, so 64 preserves useful policy depth while bounding stack use.
+const MAX_MINISCRIPT_ENCODE_DEPTH: usize = 64;
+
+fn validate_miniscript_depth<Pk, Ctx>(
+ miniscript: &miniscript::Miniscript<Pk, Ctx>,
+) -> Result<(), Error>
+where
+ Pk: miniscript::MiniscriptKey,
+ Ctx: miniscript::ScriptContext,
+{
+ // tree_height counts edges, while encoding also creates a frame for the root.
+ if miniscript.ext.tree_height >= MAX_MINISCRIPT_ENCODE_DEPTH {
+ return Err(Error::InvalidInput);
+ }
+ Ok(())
+}
+
// We only support Bitcoin for now.
fn check_enabled(coin: BtcCoin) -> Result<(), Error> {
if !matches!(coin, BtcCoin::Btc | BtcCoin::Tbtc) {
@@ -704,6 +722,7 @@ pub async fn parse<'a>(
let miniscript_expr: miniscript::Miniscript<String, miniscript::Segwitv0> =
miniscript::Miniscript::from_str(&desc[4..desc.len() - 1])
.or(Err(Error::InvalidInput))?;
+ validate_miniscript_depth(&miniscript_expr)?;
miniscript_expr
.sanity_check()
.map_err(|_| Error::InvalidInput)?;
@@ -719,6 +738,9 @@ pub async fn parse<'a>(
// calls the equivalent of the sanity check. We call it anyway below in case the
// miniscript library extends/changes the main sanity_check function.
let tr = miniscript::descriptor::Tr::from_str(desc).map_err(|_| Error::InvalidInput)?;
+ for leaf in tr.leaves() {
+ validate_miniscript_depth(leaf.miniscript())?;
+ }
tr.sanity_check().map_err(|_| Error::InvalidInput)?;
ParsedPolicy {
@@ -913,6 +935,93 @@ mod tests {
}
}
+ #[test]
+ fn test_validate_miniscript_depth() {
+ let miniscript: miniscript::Miniscript<String, miniscript::Segwitv0> =
+ miniscript::Miniscript::from_str(&format!(
+ "{}:pk(A)",
+ "n".repeat(MAX_MINISCRIPT_ENCODE_DEPTH - 2)
+ ))
+ .unwrap();
+ assert_eq!(miniscript.ext.tree_height + 1, MAX_MINISCRIPT_ENCODE_DEPTH);
+ assert_eq!(validate_miniscript_depth(&miniscript), Ok(()));
+
+ let miniscript: miniscript::Miniscript<String, miniscript::Segwitv0> =
+ miniscript::Miniscript::from_str(&format!(
+ "{}:pk(A)",
+ "n".repeat(MAX_MINISCRIPT_ENCODE_DEPTH - 1)
+ ))
+ .unwrap();
+ assert_eq!(
+ miniscript.ext.tree_height + 1,
+ MAX_MINISCRIPT_ENCODE_DEPTH + 1
+ );
+ assert_eq!(
+ validate_miniscript_depth(&miniscript),
+ Err(Error::InvalidInput)
+ );
+ }
+
+ #[async_test::test]
+ async fn test_parse_rejects_deep_miniscript() {
+ mock_unlocked();
+ let coin = BtcCoin::Tbtc;
+ let our_key = make_our_key(KEYPATH_ACCOUNT).await;
+ let accepted_leaf = format!("{}:pk(@1/**)", "n".repeat(MAX_MINISCRIPT_ENCODE_DEPTH - 2));
+ let rejected_leaf = format!("{}:pk(@1/**)", "n".repeat(MAX_MINISCRIPT_ENCODE_DEPTH - 1));
+
+ for (descriptor, keys) in [
+ (
+ format!("wsh({})", accepted_leaf.replace("@1", "@0")),
+ vec![our_key.clone()],
+ ),
+ (
+ format!("tr(@0/**,{accepted_leaf})"),
+ vec![our_key.clone(), make_key(SOME_XPUB_1)],
+ ),
+ ] {
+ let policy = make_policy(&descriptor, &keys);
+ assert!(
+ parse(&mut crate::hal::testing::TestingHal::new(), &policy, coin)
+ .await
+ .is_ok()
+ );
+ }
+
+ for (descriptor, keys) in [
+ (
+ format!("wsh({})", rejected_leaf.replace("@1", "@0")),
+ vec![our_key.clone()],
+ ),
+ (
+ format!("tr(@0/**,{rejected_leaf})"),
+ vec![our_key.clone(), make_key(SOME_XPUB_1)],
+ ),
+ ] {
+ let policy = make_policy(&descriptor, &keys);
+ assert!(
+ parse(&mut crate::hal::testing::TestingHal::new(), &policy, coin)
+ .await
+ .is_err()
+ );
+ }
+
+ let deep_policy = format!("wsh({}{}:pk(@0/**))", "n".repeat(155), "tv".repeat(45));
+ let deep_miniscript: miniscript::Miniscript<String, miniscript::Segwitv0> =
+ miniscript::Miniscript::from_str(&deep_policy[4..deep_policy.len() - 1]).unwrap();
+ assert!(deep_miniscript.sanity_check().is_ok());
+ assert_eq!(
+ validate_miniscript_depth(&deep_miniscript),
+ Err(Error::InvalidInput)
+ );
+ let policy = make_policy(&deep_policy, &[our_key]);
+ assert!(
+ parse(&mut crate::hal::testing::TestingHal::new(), &policy, coin)
+ .await
+ .is_err()
+ );
+ }
+
#[async_test::test]
async fn test_parse_wsh_miniscript() {
let coin = BtcCoin::Tbtc;
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.