Merge #714: Backport to 10.x of #712

32bd66c Release 10.2 (Sanket Kanjalkar)
708b89c Fix panic while decoding large Miniscripts from Script (Sanket Kanjalkar)

Pull request description:

ACKs for top commit:
  apoelstra:
    ACK 32bd66c

Tree-SHA512: 946ea92a680cec1a7ccfc78138a4dbefed7421aac013f68bee3730882741eb1319a8ea3d351fe663d8881e84f0108460a65476e4371d65359e8a968ffe3b806b

Author: apoelstra

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)

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "miniscript"
-version = "10.1.0"
+version = "10.2.0"
 authors = ["Andrew Poelstra <[email protected]>, Sanket Kanjalkar <[email protected]>"]
 license = "CC0-1.0"
 homepage = "https://github.com/rust-bitcoin/rust-miniscript/"

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!(

src/miniscript/mod.rs

@@ -13,15 +13,14 @@
 //! components of the AST.
 //!
 
-use core::marker::PhantomData;
 use core::{fmt, hash, str};
 
 use bitcoin::script;
 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,54 +149,14 @@ 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)
     }
 }
 
 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();
+    }
 }