Skip to content

Commit

Permalink
Merge #713: Fix panic while decoding large Miniscripts from Script
Browse files Browse the repository at this point in the history 
ec8b8c3 Release 11.2 (Sanket Kanjalkar)
7b656b9 Fix panic while decoding large Miniscripts from Script (Sanket Kanjalkar)

Pull request description:

  Backport 11.x

ACKs for top commit:
  apoelstra:
    ACK ec8b8c3

Tree-SHA512: d03620f971f8f23cc48816568b557f6de4f862dd254b1d6e494f15e9e42cb0004f81baeca44d0dbd49384860bd2f6b75402b61bd26d299df3ac400f9b81e5eaf
  • Loading branch information
@apoelstra
apoelstra committed Jul 23, 2024
2 parents 2f1a262 + ec8b8c3 commit 2c9d2e7
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 99 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "miniscript"
version = "11.1.0"
version = "11.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/"
Expand Down
24 changes: 4 additions & 20 deletions src/miniscript/decode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//!
use core::fmt;
use core::marker::PhantomData;
#[cfg(feature = "std")]
use std::error;

Expand All @@ -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)]
Expand Down Expand Up @@ -212,10 +209,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)?;
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(())
}
Expand All @@ -228,10 +222,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)?;
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(())
}
Expand All @@ -246,10 +237,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)?;
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(())
}
Expand Down Expand Up @@ -531,11 +519,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)?;
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!(
Expand Down
156 changes: 78 additions & 78 deletions src/miniscript/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
//! components of the AST.
//!
use core::marker::PhantomData;
use core::{fmt, hash, str};

use bitcoin::hashes::hash160;
Expand All @@ -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;
Expand All @@ -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<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>,
}
mod private {
use core::marker::PhantomData;

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)?,
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<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)?,
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<Pk, Ctx>,
ty: types::Type,
ext: types::extra_props::ExtData,
) -> Miniscript<Pk, Ctx> {
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<Pk, Ctx>,
ty: types::Type,
ext: types::extra_props::ExtData,
) -> Miniscript<Pk, Ctx> {
Miniscript { node, ty, ext, phantom: PhantomData }
}
}
}

pub use private::Miniscript;

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 }

Expand Down Expand Up @@ -603,7 +613,6 @@ pub mod hash256 {
#[cfg(test)]
mod tests {

use core::marker::PhantomData;
use core::str;
use core::str::FromStr;

Expand All @@ -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};
Expand Down Expand Up @@ -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::<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(\"\")";
Expand All @@ -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::<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(
Expand All @@ -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::<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");
}
Expand Down Expand Up @@ -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();
}
}

0 comments on commit 2c9d2e7

Please sign in to comment.