From 7b656b9ee4e6a7ba1ec42775f45465b765119244 Mon Sep 17 00:00:00 2001 From: Sanket Kanjalkar 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 | 24 +----- src/miniscript/mod.rs | 156 +++++++++++++++++++-------------------- 2 files changed, 82 insertions(+), 98 deletions(-) diff --git a/src/miniscript/decode.rs b/src/miniscript/decode.rs index 409563b04..d0ce937c7 100644 --- a/src/miniscript/decode.rs +++ b/src/miniscript/decode.rs @@ -6,7 +6,6 @@ //! use core::fmt; -use core::marker::PhantomData; #[cfg(feature = "std")] use std::error; @@ -16,8 +15,6 @@ 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::*; #[cfg(doc)] @@ -212,10 +209,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)?; - let ext = ExtData::type_check(&ms)?; - 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(()) } @@ -228,10 +222,7 @@ impl TerminalStack { let top = self.pop().unwrap(); let wrapped_ms = wrap(Arc::new(top)); - let ty = Type::type_check(&wrapped_ms)?; - let ext = ExtData::type_check(&wrapped_ms)?; - 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(()) } @@ -246,10 +237,7 @@ impl TerminalStack { let wrapped_ms = wrap(Arc::new(left), Arc::new(right)); - let ty = Type::type_check(&wrapped_ms)?; - let ext = ExtData::type_check(&wrapped_ms)?; - 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(()) } @@ -531,11 +519,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)?; - let ext = ExtData::type_check(&wrapped_ms)?; - - 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 cdef49807..a6b80d43b 100644 --- a/src/miniscript/mod.rs +++ b/src/miniscript/mod.rs @@ -12,7 +12,6 @@ //! components of the AST. //! -use core::marker::PhantomData; use core::{fmt, hash, str}; use bitcoin::hashes::hash160; @@ -22,7 +21,7 @@ use bitcoin::taproot::{LeafVersion, TapLeafHash}; use self::analyzable::ExtParams; pub use self::context::{BareCtx, Legacy, Segwitv0, Tap}; use crate::iter::TreeLike; -use crate::{prelude::*, MAX_RECURSION_DEPTH}; +use crate::prelude::*; use crate::{script_num_size, TranslateErr}; pub mod analyzable; @@ -43,62 +42,73 @@ 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, plan, Error, ForEachKey, MiniscriptKey, ToPublicKey, TranslatePk, Translator, }; #[cfg(test)] mod ms_tests; -/// 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, -} +mod private { + use core::marker::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)?, - ext: ExtData::type_check(&t)?, - 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) + 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)?, + ext: ExtData::type_check(&t)?, + 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 } + /// 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; +impl Miniscript { /// Extracts the `AstElem` representing the root of the miniscript pub fn into_inner(self) -> Terminal { self.node } @@ -603,7 +613,6 @@ pub mod hash256 { #[cfg(test)] mod tests { - use core::marker::PhantomData; use core::str; use core::str::FromStr; @@ -614,7 +623,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}; @@ -800,21 +809,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::(), - ext: types::extra_props::ExtData::from_pk_k::(), - phantom: PhantomData, - })); + let pk_node = Terminal::Check(Arc::new( + Miniscript::from_ast(Terminal::PkK(String::from(""))).unwrap(), + )); let pkk_ms: Miniscript = 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::(), - ext: types::extra_props::ExtData::from_pk_h::(), - 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(\"\")"; @@ -830,12 +833,7 @@ mod tests { assert_eq!(display, expected); } - let pkk_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, - })); + 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( @@ -844,17 +842,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"); } @@ -1366,4 +1357,13 @@ mod tests { Err(Error::MaxRecursiveDepthExceeded) ); } + + #[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 ec8b8c36f6270b456f4a34c94d24a1c88e86e7ab Mon Sep 17 00:00:00 2001 From: Sanket Kanjalkar Date: Sat, 20 Jul 2024 18:47:11 -0700 Subject: [PATCH 2/2] Release 11.2 --- CHANGELOG.md | 3 +++ Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92cd79f8d..68255202e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 11.2.0 - July 20, 2024 + +- Fix panics while decoding large miniscripts from script [#712](https://github.com/rust-bitcoin/rust-miniscript/pull/712) # 11.1.0 - July 9, 2024 diff --git a/Cargo.toml b/Cargo.toml index 184585bcf..f57dc899d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "miniscript" -version = "11.1.0" +version = "11.2.0" authors = ["Andrew Poelstra , Sanket Kanjalkar "] license = "CC0-1.0" homepage = "https://github.com/rust-bitcoin/rust-miniscript/"