From 708b89cfad9c925ab418774359b238f6335fe0aa Mon Sep 17 00:00:00 2001
From: Sanket Kanjalkar <sanketk@squareup.com>
Date: Sat, 20 Jul 2024 18:30:10 -0700
Subject: [PATCH 1/2] Fix panic while decoding large Miniscripts from Script
---
src/miniscript/decode.rs | 50 ++----------
src/miniscript/mod.rs | 168 +++++++++++++++++++--------------------
2 files changed, 87 insertions(+), 131 deletions(-)
diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs
index dbf4adcfe..252b24f1d 100644
--- a/src/miniscript/decode.rs
+++ b/src/miniscript/decode.rs
@@ -7,7 +7,6 @@
//!
use core::fmt;
-use core::marker::PhantomData;
#[cfg(feature = "std")]
use std::error;
@@ -18,17 +17,12 @@ use sync::Arc;
use crate::miniscript::lex::{Token as Tk, TokenIter};
use crate::miniscript::limits::MAX_PUBKEYS_PER_MULTISIG;
-use crate::miniscript::types::extra_props::ExtData;
-use crate::miniscript::types::{Property, Type};
use crate::miniscript::ScriptContext;
-use crate::prelude::*;
+use crate::{prelude::*, Miniscript};
#[cfg(doc)]
use crate::Descriptor;
-use crate::{bitcoin, hash256, AbsLockTime, Error, Miniscript, MiniscriptKey, ToPublicKey};
+use crate::{bitcoin, hash256, AbsLockTime, Error, MiniscriptKey, ToPublicKey};
-fn return_none<T>(_: usize) -> Option<T> {
- None
-}
/// Trait for parsing keys from byte slices
pub trait ParseableKey: Sized + ToPublicKey + private::Sealed {
@@ -224,15 +218,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
///reduce, type check and push a 0-arg node
fn reduce0(&mut self, ms: Terminal<Pk, Ctx>) -> Result<(), Error> {
- let ty = Type::type_check(&ms, return_none)?;
- let ext = ExtData::type_check(&ms, return_none)?;
- let ms = Miniscript {
- node: ms,
- ty,
- ext,
- phantom: PhantomData,
- };
- Ctx::check_global_validity(&ms)?;
+ let ms = Miniscript::from_ast(ms)?;
self.0.push(ms);
Ok(())
}
@@ -245,15 +231,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
let top = self.pop().unwrap();
let wrapped_ms = wrap(Arc::new(top));
- let ty = Type::type_check(&wrapped_ms, return_none)?;
- let ext = ExtData::type_check(&wrapped_ms, return_none)?;
- let ms = Miniscript {
- node: wrapped_ms,
- ty,
- ext,
- phantom: PhantomData,
- };
- Ctx::check_global_validity(&ms)?;
+ let ms = Miniscript::from_ast(wrapped_ms)?;
self.0.push(ms);
Ok(())
}
@@ -268,15 +246,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> TerminalStack<Pk, Ctx> {
let wrapped_ms = wrap(Arc::new(left), Arc::new(right));
- let ty = Type::type_check(&wrapped_ms, return_none)?;
- let ext = ExtData::type_check(&wrapped_ms, return_none)?;
- let ms = Miniscript {
- node: wrapped_ms,
- ty,
- ext,
- phantom: PhantomData,
- };
- Ctx::check_global_validity(&ms)?;
+ let ms = Miniscript::from_ast(wrapped_ms)?;
self.0.push(ms);
Ok(())
}
@@ -557,15 +527,7 @@ pub fn parse<Ctx: ScriptContext>(
let c = term.pop().unwrap();
let wrapped_ms = Terminal::AndOr(Arc::new(a), Arc::new(c), Arc::new(b));
- let ty = Type::type_check(&wrapped_ms, return_none)?;
- let ext = ExtData::type_check(&wrapped_ms, return_none)?;
-
- term.0.push(Miniscript {
- node: wrapped_ms,
- ty,
- ext,
- phantom: PhantomData,
- });
+ term.0.push(Miniscript::from_ast(wrapped_ms)?);
}
Some(NonTerm::ThreshW { n, k }) => {
match_token!(
diff --git a/src/miniscript/mod.rs b/src/miniscript/mod.rs
index 7e877b7c8..a5959081d 100644
--- a/src/miniscript/mod.rs
+++ b/src/miniscript/mod.rs
@@ -13,7 +13,6 @@
//! components of the AST.
//!
-use core::marker::PhantomData;
use core::{fmt, hash, str};
use bitcoin::script;
@@ -21,7 +20,7 @@ use bitcoin::taproot::{LeafVersion, TapLeafHash};
use self::analyzable::ExtParams;
pub use self::context::{BareCtx, Legacy, Segwitv0, Tap};
-use crate::{prelude::*, MAX_RECURSION_DEPTH};
+use crate::prelude::*;
use crate::TranslateErr;
pub mod analyzable;
@@ -42,25 +41,70 @@ use self::lex::{lex, TokenIter};
use self::types::Property;
pub use crate::miniscript::context::ScriptContext;
use crate::miniscript::decode::Terminal;
-use crate::miniscript::types::extra_props::ExtData;
-use crate::miniscript::types::Type;
use crate::{expression, Error, ForEachKey, MiniscriptKey, ToPublicKey, TranslatePk, Translator};
#[cfg(test)]
mod ms_tests;
-/// Top-level script AST type
-#[derive(Clone)]
-pub struct Miniscript<Pk: MiniscriptKey, Ctx: ScriptContext> {
- ///A node in the Abstract Syntax Tree(
- pub node: Terminal<Pk, Ctx>,
- ///The correctness and malleability type information for the AST node
- pub ty: types::Type,
- ///Additional information helpful for extra analysis.
- pub ext: types::extra_props::ExtData,
- /// Context PhantomData. Only accessible inside this crate
- phantom: PhantomData<Ctx>,
+mod private {
+ use core::marker::PhantomData;
+
+ use super::types::{ExtData, Property, Type};
+ pub use crate::miniscript::context::ScriptContext;
+ use crate::miniscript::types;
+ use crate::{Error, MiniscriptKey, Terminal, MAX_RECURSION_DEPTH};
+
+ /// The top-level miniscript abstract syntax tree (AST).
+ #[derive(Clone)]
+ pub struct Miniscript<Pk: MiniscriptKey, Ctx: ScriptContext> {
+ /// A node in the AST.
+ pub node: Terminal<Pk, Ctx>,
+ /// The correctness and malleability type information for the AST node.
+ pub ty: types::Type,
+ /// Additional information helpful for extra analysis.
+ pub ext: types::extra_props::ExtData,
+ /// Context PhantomData. Only accessible inside this crate
+ phantom: PhantomData<Ctx>,
+ }
+ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
+
+ /// Add type information(Type and Extdata) to Miniscript based on
+ /// `AstElem` fragment. Dependent on display and clone because of Error
+ /// Display code of type_check.
+ pub fn from_ast(t: Terminal<Pk, Ctx>) -> Result<Miniscript<Pk, Ctx>, Error> {
+ let res = Miniscript {
+ ty: Type::type_check(&t, |_| None)?,
+ ext: ExtData::type_check(&t, |_| None)?,
+ node: t,
+ phantom: PhantomData,
+ };
+ // TODO: This recursion depth is based on segwitv0.
+ // We can relax this in tapscript, but this should be good for almost
+ // all practical cases and we can revisit this if needed.
+ // casting to u32 is safe because tree_height will never go more than u32::MAX
+ if (res.ext.tree_height as u32) > MAX_RECURSION_DEPTH {
+ return Err(Error::MaxRecursiveDepthExceeded);
+ }
+ Ctx::check_global_consensus_validity(&res)?;
+ Ok(res)
+ }
+
+ /// Create a new `Miniscript` from a `Terminal` node and a `Type` annotation
+ /// This does not check the typing rules. The user is responsible for ensuring
+ /// that the type provided is correct.
+ ///
+ /// You should almost always use `Miniscript::from_ast` instead of this function.
+ pub fn from_components_unchecked(
+ node: Terminal<Pk, Ctx>,
+ ty: types::Type,
+ ext: types::extra_props::ExtData,
+ ) -> Miniscript<Pk, Ctx> {
+ Miniscript { node, ty, ext, phantom: PhantomData }
+ }
+ }
}
+pub use private::Miniscript;
+
/// `PartialOrd` of `Miniscript` must depend only on node and not the type information.
/// The type information and extra_properties can be deterministically determined
/// by the ast.
@@ -105,47 +149,6 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> hash::Hash for Miniscript<Pk, Ctx> {
}
}
-impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
- /// Add type information(Type and Extdata) to Miniscript based on
- /// `AstElem` fragment. Dependent on display and clone because of Error
- /// Display code of type_check.
- pub fn from_ast(t: Terminal<Pk, Ctx>) -> Result<Miniscript<Pk, Ctx>, Error> {
- let res = Miniscript {
- ty: Type::type_check(&t, |_| None)?,
- ext: ExtData::type_check(&t, |_| None)?,
- node: t,
- phantom: PhantomData,
- };
- // TODO: This recursion depth is based on segwitv0.
- // We can relax this in tapscript, but this should be good for almost
- // all practical cases and we can revisit this if needed.
- // casting to u32 is safe because tree_height will never go more than u32::MAX
- if (res.ext.tree_height as u32) > MAX_RECURSION_DEPTH {
- return Err(Error::MaxRecursiveDepthExceeded);
- }
- Ctx::check_global_consensus_validity(&res)?;
- Ok(res)
- }
-
- /// Create a new `Miniscript` from a `Terminal` node and a `Type` annotation
- /// This does not check the typing rules. The user is responsible for ensuring
- /// that the type provided is correct.
- ///
- /// You should almost always use `Miniscript::from_ast` instead of this function.
- pub fn from_components_unchecked(
- node: Terminal<Pk, Ctx>,
- ty: types::Type,
- ext: types::extra_props::ExtData,
- ) -> Miniscript<Pk, Ctx> {
- Miniscript {
- node,
- ty,
- ext,
- phantom: PhantomData,
- }
- }
-}
-
impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Display for Miniscript<Pk, Ctx> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.node)
@@ -153,6 +156,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Display for Miniscript<Pk, Ctx>
}
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
+
/// Extracts the `AstElem` representing the root of the miniscript
pub fn into_inner(self) -> Terminal<Pk, Ctx> {
self.node
@@ -481,7 +485,6 @@ pub mod hash256 {
#[cfg(test)]
mod tests {
- use core::marker::PhantomData;
use core::str;
use core::str::FromStr;
@@ -492,7 +495,7 @@ mod tests {
use sync::Arc;
use super::{Miniscript, ScriptContext, Segwitv0, Tap};
- use crate::miniscript::types::{self, ExtData, Property, Type};
+ use crate::miniscript::types;
use crate::miniscript::Terminal;
use crate::policy::Liftable;
use crate::{prelude::*, Error};
@@ -678,21 +681,15 @@ mod tests {
.unwrap();
let hash = hash160::Hash::from_byte_array([17; 20]);
- let pk_node = Terminal::Check(Arc::new(Miniscript {
- node: Terminal::PkK(String::from("")),
- ty: Type::from_pk_k::<Segwitv0>(),
- ext: types::extra_props::ExtData::from_pk_k::<Segwitv0>(),
- phantom: PhantomData,
- }));
+ let pk_node = Terminal::Check(Arc::new(
+ Miniscript::from_ast(Terminal::PkK(String::from(""))).unwrap(),
+ ));
let pkk_ms: Miniscript<String, Segwitv0> = Miniscript::from_ast(pk_node).unwrap();
dummy_string_rtt(pkk_ms, "[B/onduesm]c:[K/onduesm]pk_k(\"\")", "pk()");
- let pkh_node = Terminal::Check(Arc::new(Miniscript {
- node: Terminal::PkH(String::from("")),
- ty: Type::from_pk_h::<Segwitv0>(),
- ext: types::extra_props::ExtData::from_pk_h::<Segwitv0>(),
- phantom: PhantomData,
- }));
+ let pkh_node = Terminal::Check(Arc::new(
+ Miniscript::from_ast(Terminal::PkH(String::from(""))).unwrap(),
+ ));
let pkh_ms: Miniscript<String, Segwitv0> = Miniscript::from_ast(pkh_node).unwrap();
let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(\"\")";
@@ -708,12 +705,7 @@ mod tests {
assert_eq!(display, expected);
}
- let pkk_node = Terminal::Check(Arc::new(Miniscript {
- node: Terminal::PkK(pk),
- ty: Type::from_pk_k::<Segwitv0>(),
- ext: types::extra_props::ExtData::from_pk_k::<Segwitv0>(),
- phantom: PhantomData,
- }));
+ let pkk_node = Terminal::Check(Arc::new(Miniscript::from_ast(Terminal::PkK(pk)).unwrap()));
let pkk_ms: Segwitv0Script = Miniscript::from_ast(pkk_node).unwrap();
script_rtt(
@@ -722,17 +714,10 @@ mod tests {
202020202ac",
);
- let pkh_ms: Segwitv0Script = Miniscript {
- node: Terminal::Check(Arc::new(Miniscript {
- node: Terminal::RawPkH(hash),
- ty: Type::from_pk_h::<Segwitv0>(),
- ext: types::extra_props::ExtData::from_pk_h::<Segwitv0>(),
- phantom: PhantomData,
- })),
- ty: Type::cast_check(Type::from_pk_h::<Segwitv0>()).unwrap(),
- ext: ExtData::cast_check(ExtData::from_pk_h::<Segwitv0>()).unwrap(),
- phantom: PhantomData,
- };
+ let pkh_ms: Segwitv0Script = Miniscript::from_ast(Terminal::Check(Arc::new(
+ Miniscript::from_ast(Terminal::RawPkH(hash)).unwrap(),
+ )))
+ .unwrap();
script_rtt(pkh_ms, "76a914111111111111111111111111111111111111111188ac");
}
@@ -1160,4 +1145,13 @@ mod tests {
panic!("Unexpected error: {:?}", err);
}
}
+
+ #[test]
+ fn test_script_parse_dos() {
+ let mut script = bitcoin::script::Builder::new().push_opcode(bitcoin::opcodes::OP_TRUE);
+ for _ in 0..10000 {
+ script = script.push_opcode(bitcoin::opcodes::all::OP_0NOTEQUAL);
+ }
+ Tapscript::parse_insane(&script.into_script()).unwrap_err();
+ }
}
From 32bd66cfe65f245462b927fadbd5a6387b736946 Mon Sep 17 00:00:00 2001
From: Sanket Kanjalkar <sanketk@squareup.com>
Date: Sat, 20 Jul 2024 19:08:41 -0700
Subject: [PATCH 2/2] Release 10.2
---
CHANGELOG.md | 4 ++++
Cargo.toml | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 32de521e6..8dbf4dd6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# # 10.2.0 - July 20, 2024
+
+- Fix panics while decoding large miniscripts from script [#712](https://github.com/rust-bitcoin/rust-miniscript/pull/712)
+
# 10.1.0 - July 9, 2024
- Explicitly track recursion depth in fragments [#704](https://github.com/rust-bitcoin/rust-miniscript/pull/704)
diff --git a/Cargo.toml b/Cargo.toml
index f99b0e6ac..ca8c7d6d9 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "miniscript"
-version = "10.1.0"
+version = "10.2.0"
authors = ["Andrew Poelstra <apoelstra@wpsoftware.net>, Sanket Kanjalkar <sanket1729@gmail.com>"]
license = "CC0-1.0"
homepage = "https://github.com/rust-bitcoin/rust-miniscript/"