diff --git a/CHANGELOG.md b/CHANGELOG.md index de9e693ff..5423c9777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 9.2.0 - July 20, 2024 + +- Fix panics while decoding large miniscripts from script [#712](https://github.com/rust-bitcoin/rust-miniscript/pull/712) + # 9.1.0 - July 13, 2023 - 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 0f360a675..74bdc567a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miniscript" -version = "9.1.0" +version = "9.2.0" authors = ["Andrew Poelstra , Sanket Kanjalkar "] license = "CC0-1.0" homepage = "https://github.com/rust-bitcoin/rust-miniscript/" diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index 7715dd051..472b35702 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -18,7 +18,6 @@ //! use core::fmt; -use core::marker::PhantomData; #[cfg(feature = "std")] use std::error; @@ -29,15 +28,10 @@ use sync::Arc; use crate::bitcoin::{LockTime, PackedLockTime, Sequence}; 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::{bitcoin, hash256, Error, Miniscript, MiniscriptKey, ToPublicKey}; -fn return_none(_: usize) -> Option { - None -} /// Trait for parsing keys from byte slices pub trait ParseableKey: Sized + ToPublicKey + private::Sealed { @@ -233,15 +227,7 @@ impl TerminalStack { ///reduce, type check and push a 0-arg node fn reduce0(&mut self, ms: Terminal) -> 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(()) } @@ -254,15 +240,7 @@ impl TerminalStack { 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(()) } @@ -277,15 +255,7 @@ impl TerminalStack { 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(()) } @@ -566,15 +536,7 @@ pub fn parse( 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 c2f17c3da..8fb3d4988 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -24,7 +24,6 @@ //! components of the AST. //! -use core::marker::PhantomData; use core::{fmt, hash, str}; use bitcoin::blockdata::script; @@ -32,7 +31,7 @@ use bitcoin::util::taproot::{LeafVersion, TapLeafHash}; use self::analyzable::ExtParams; pub use self::context::{BareCtx, Legacy, Segwitv0, Tap}; -use crate::{prelude::*, MAX_RECURSION_DEPTH}; +use crate::prelude::*; pub mod analyzable; pub mod astelem; @@ -53,25 +52,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 { - ///A node in the Abstract Syntax Tree( - pub node: Terminal, - ///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 - pub(crate) phantom: PhantomData, +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 { + /// A node in the AST. + pub node: Terminal, + /// 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, + } + impl Miniscript { + + /// 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) -> Result, 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, + ty: types::Type, + ext: types::extra_props::ExtData, + ) -> Miniscript { + 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. @@ -116,29 +160,6 @@ impl hash::Hash for Miniscript { } } -impl Miniscript { - /// 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) -> Result, 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 { - Err(Error::MaxRecursiveDepthExceeded) - } else { - Ok(res) - } - } -} - impl fmt::Display for Miniscript { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.node) @@ -146,6 +167,7 @@ impl fmt::Display for Miniscript } impl Miniscript { + /// Extracts the `AstElem` representing the root of the miniscript pub fn into_inner(self) -> Terminal { self.node @@ -338,14 +360,8 @@ impl Miniscript { T: Translator, { let inner = self.node.real_translate_pk(t)?; - let ms = Miniscript { - //directly copying the type and ext is safe because translating public - //key should not change any properties - ty: self.ty, - ext: self.ext, - node: inner, - phantom: PhantomData, - }; + let ms = Miniscript::from_ast(inner) + .expect("Translator should not change the type of the AST"); Ok(ms) } } @@ -455,12 +471,7 @@ impl_from_tree!( /// should not be called directly; rather go through the descriptor API. fn from_tree(top: &expression::Tree) -> Result, Error> { let inner: Terminal = expression::FromTree::from_tree(top)?; - Ok(Miniscript { - ty: Type::type_check(&inner, |_| None)?, - ext: ExtData::type_check(&inner, |_| None)?, - node: inner, - phantom: PhantomData, - }) + Ok(Miniscript::from_ast(inner)?) } ); @@ -482,7 +493,6 @@ serde_string_impl_pk!(Miniscript, "a miniscript", Ctx; ScriptContext); #[cfg(test)] mod tests { - use core::marker::PhantomData; use core::str; use core::str::FromStr; @@ -493,7 +503,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}; @@ -679,32 +689,18 @@ mod tests { .unwrap(); let hash = hash160::Hash::from_inner([17; 20]); - let pkk_ms: Miniscript = Miniscript { - node: Terminal::Check(Arc::new(Miniscript { - node: Terminal::PkK(DummyKey), - ty: Type::from_pk_k::(), - ext: types::extra_props::ExtData::from_pk_k::(), - phantom: PhantomData, - })), - ty: Type::cast_check(Type::from_pk_k::()).unwrap(), - ext: ExtData::cast_check(ExtData::from_pk_k::()).unwrap(), - phantom: PhantomData, - }; + let pk_node = Terminal::Check(Arc::new( + Miniscript::from_ast(Terminal::PkK(DummyKey)).unwrap(), + )); + let pkk_ms: Miniscript = Miniscript::from_ast(pk_node).unwrap(); dummy_string_rtt(pkk_ms, "[B/onduesm]c:[K/onduesm]pk_k(DummyKey)", "pk()"); - let pkh_ms: Miniscript = Miniscript { - node: Terminal::Check(Arc::new(Miniscript { - node: Terminal::PkH(DummyKey), - ty: Type::from_pk_h::(), - ext: types::extra_props::ExtData::from_pk_h::(), - phantom: PhantomData, - })), - ty: Type::cast_check(Type::from_pk_h::()).unwrap(), - ext: ExtData::cast_check(ExtData::from_pk_h::()).unwrap(), - phantom: PhantomData, - }; + let pkh_node = Terminal::Check(Arc::new( + Miniscript::from_ast(Terminal::PkH(String::from(""))).unwrap(), + )); + let pkh_ms: Miniscript = Miniscript::from_ast(pkh_node).unwrap(); - let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(DummyKey)"; + let expected_debug = "[B/nduesm]c:[K/nduesm]pk_h(\"\")"; let expected_display = "pkh()"; assert_eq!(pkh_ms.ty.corr.base, types::Base::B); @@ -717,17 +713,8 @@ mod tests { assert_eq!(display, expected); } - let pkk_ms: Segwitv0Script = Miniscript { - node: Terminal::Check(Arc::new(Miniscript { - node: Terminal::PkK(pk), - ty: Type::from_pk_k::(), - ext: types::extra_props::ExtData::from_pk_k::(), - phantom: PhantomData, - })), - ty: Type::cast_check(Type::from_pk_k::()).unwrap(), - ext: ExtData::cast_check(ExtData::from_pk_k::()).unwrap(), - 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( pkk_ms, @@ -735,17 +722,10 @@ mod tests { 202020202ac", ); - let pkh_ms: Segwitv0Script = Miniscript { - node: Terminal::Check(Arc::new(Miniscript { - node: Terminal::RawPkH(hash), - ty: Type::from_pk_h::(), - ext: types::extra_props::ExtData::from_pk_h::(), - phantom: PhantomData, - })), - ty: Type::cast_check(Type::from_pk_h::()).unwrap(), - ext: ExtData::cast_check(ExtData::from_pk_h::()).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 +1140,13 @@ mod tests { panic!("Unexpected error: {:?}", err); } } + + #[test] + fn test_script_parse_dos() { + let mut script = bitcoin::blockdata::script::Builder::new().push_opcode(bitcoin::blockdata::opcodes::OP_TRUE); + for _ in 0..10000 { + script = script.push_opcode(bitcoin::blockdata::opcodes::all::OP_0NOTEQUAL); + } + Tapscript::parse_insane(&script.into_script()).unwrap_err(); + } } diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index e7269d170..77febf917 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -18,7 +18,6 @@ //! use core::convert::From; -use core::marker::PhantomData; use core::{cmp, f64, fmt, hash, mem}; #[cfg(feature = "std")] use std::error; @@ -507,12 +506,9 @@ impl AstElemExt { let ext = types::ExtData::type_check(&ast, |_| None)?; let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?; Ok(AstElemExt { - ms: Arc::new(Miniscript { - ty, - ext, - node: ast, - phantom: PhantomData, - }), + ms: Arc::new( + Miniscript::from_components_unchecked(ast, ty, ext) + ), comp_ext_data, }) } @@ -535,12 +531,9 @@ impl AstElemExt { let ext = types::ExtData::type_check(&ast, |_| None)?; let comp_ext_data = CompilerExtData::type_check(&ast, lookup_ext)?; Ok(AstElemExt { - ms: Arc::new(Miniscript { - ty, - ext, - node: ast, - phantom: PhantomData, - }), + ms: Arc::new( + Miniscript::from_components_unchecked(ast, ty, ext) + ), comp_ext_data, }) } @@ -558,12 +551,11 @@ struct Cast { impl Cast { fn cast(&self, ast: &AstElemExt) -> Result, ErrorKind> { Ok(AstElemExt { - ms: Arc::new(Miniscript { - ty: (self.ast_type)(ast.ms.ty)?, - ext: (self.ext_data)(ast.ms.ext)?, - node: (self.node)(Arc::clone(&ast.ms)), - phantom: PhantomData, - }), + ms: Arc::new(Miniscript::from_components_unchecked( + (self.node)(Arc::clone(&ast.ms)), + (self.ast_type)(ast.ms.ty)?, + (self.ext_data)(ast.ms.ext)?, + )), comp_ext_data: (self.comp_ext_data)(ast.comp_ext_data)?, }) } @@ -1011,8 +1003,7 @@ where let ast = Terminal::Thresh(k, sub_ast); let ast_ext = AstElemExt { ms: Arc::new( - Miniscript::from_ast(ast) - .expect("threshold subs, which we just compiled, typeck"), + Miniscript::from_ast(ast).map_err(|_| CompilerError::LimitsExceeded)?, ), comp_ext_data: CompilerExtData::threshold(k, n, |i| Ok(sub_ext_data[i])) .expect("threshold subs, which we just compiled, typeck"), diff --git a/src/policy/semantic.rs b/src/policy/semantic.rs index 62a71f1be..8239de456 100644 --- a/src/policy/semantic.rs +++ b/src/policy/semantic.rs @@ -739,7 +739,7 @@ mod tests { ) ); assert_eq!(policy.relative_timelocks(), vec![1000]); - assert_eq!(policy.absolute_timelocks(), vec![]); + assert_eq!(policy.absolute_timelocks(), Vec::::new()); assert_eq!( policy.clone().at_age(Sequence::ZERO), Policy::Key("".to_owned()) @@ -764,8 +764,8 @@ mod tests { policy, Policy::Threshold(1, vec![Policy::Key("".to_owned()), Policy::Unsatisfiable,]) ); - assert_eq!(policy.relative_timelocks(), vec![]); - assert_eq!(policy.absolute_timelocks(), vec![]); + assert_eq!(policy.relative_timelocks(), Vec::::new()); + assert_eq!(policy.absolute_timelocks(), Vec::::new()); assert_eq!(policy.n_keys(), 1); assert_eq!(policy.minimum_n_keys(), Some(1)); @@ -774,8 +774,8 @@ mod tests { policy, Policy::Threshold(2, vec![Policy::Key("".to_owned()), Policy::Unsatisfiable,]) ); - assert_eq!(policy.relative_timelocks(), vec![]); - assert_eq!(policy.absolute_timelocks(), vec![]); + assert_eq!(policy.relative_timelocks(), Vec::::new()); + assert_eq!(policy.absolute_timelocks(), Vec::::new()); assert_eq!(policy.n_keys(), 1); assert_eq!(policy.minimum_n_keys(), None); @@ -833,7 +833,7 @@ mod tests { let policy = StringPolicy::from_str("after(1000)").unwrap(); assert_eq!(policy, Policy::after(1000)); assert_eq!(policy.absolute_timelocks(), vec![1000]); - assert_eq!(policy.relative_timelocks(), vec![]); + assert_eq!(policy.relative_timelocks(), Vec::::new()); assert_eq!( policy.clone().at_lock_time(LockTime::ZERO), Policy::Unsatisfiable @@ -870,7 +870,7 @@ mod tests { let policy = StringPolicy::from_str("after(500000010)").unwrap(); assert_eq!(policy, Policy::after(500_000_010)); assert_eq!(policy.absolute_timelocks(), vec![500_000_010]); - assert_eq!(policy.relative_timelocks(), vec![]); + assert_eq!(policy.relative_timelocks(), Vec::::new()); // Pass a block height to at_lock_time while policy uses a UNIX timestapm. assert_eq!( policy.clone().at_lock_time(LockTime::ZERO),