feat: check max script sig push

rust-bitcoin · Sep 7, 2024 · 324b33d · 324b33d
1 parent b11cdc2
commit 324b33d
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 11 deletions.
diff --git a/src/descriptor/bare.rs b/src/descriptor/bare.rs
@@ -120,7 +120,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Bare<Pk> {
         S: Satisfier<Pk>,
     {
         let ms = self.ms.satisfy(satisfier)?;
-        let script_sig = witness_to_scriptsig(&ms);
+        let script_sig = witness_to_scriptsig(&ms)?;
         let witness = vec![];
         Ok((witness, script_sig))
     }
@@ -133,7 +133,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Bare<Pk> {
         S: Satisfier<Pk>,
     {
         let ms = self.ms.satisfy_malleable(satisfier)?;
-        let script_sig = witness_to_scriptsig(&ms);
+        let script_sig = witness_to_scriptsig(&ms)?;
         let witness = vec![];
         Ok((witness, script_sig))
     }

diff --git a/src/descriptor/sh.rs b/src/descriptor/sh.rs
@@ -376,14 +376,14 @@ impl<Pk: MiniscriptKey + ToPublicKey> Sh<Pk> {
             ShInner::SortedMulti(ref smv) => {
                 let mut script_witness = smv.satisfy(satisfier)?;
                 script_witness.push(smv.encode().into_bytes());
-                let script_sig = witness_to_scriptsig(&script_witness);
+                let script_sig = witness_to_scriptsig(&script_witness)?;
                 let witness = vec![];
                 Ok((witness, script_sig))
             }
             ShInner::Ms(ref ms) => {
                 let mut script_witness = ms.satisfy(satisfier)?;
                 script_witness.push(ms.encode().into_bytes());
-                let script_sig = witness_to_scriptsig(&script_witness);
+                let script_sig = witness_to_scriptsig(&script_witness)?;
                 let witness = vec![];
                 Ok((witness, script_sig))
             }
@@ -406,7 +406,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Sh<Pk> {
             ShInner::Ms(ref ms) => {
                 let mut script_witness = ms.satisfy_malleable(satisfier)?;
                 script_witness.push(ms.encode().into_bytes());
-                let script_sig = witness_to_scriptsig(&script_witness);
+                let script_sig = witness_to_scriptsig(&script_witness)?;
                 let witness = vec![];
                 Ok((witness, script_sig))
             }

diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs
@@ -385,7 +385,7 @@ impl ScriptContext for Legacy {
     fn check_witness(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
         // In future, we could avoid by having a function to count only
         // len of script instead of converting it.
-        if witness_to_scriptsig(witness).len() > MAX_SCRIPTSIG_SIZE {
+        if witness_to_scriptsig(witness).unwrap().len() > MAX_SCRIPTSIG_SIZE {
             return Err(ScriptContextError::MaxScriptSigSizeExceeded);
         }
         Ok(())

diff --git a/src/util.rs b/src/util.rs
@@ -2,6 +2,7 @@
 
 use core::convert::TryFrom;
 
+use bitcoin::constants::MAX_SCRIPT_ELEMENT_SIZE;
 use bitcoin::hashes::Hash;
 use bitcoin::script::{self, PushBytes, ScriptBuf};
 use bitcoin::PubkeyHash;
@@ -47,18 +48,22 @@ pub(crate) fn witness_size<T: ItemSize>(wit: &[T]) -> usize {
     wit.iter().map(T::size).sum::<usize>() + varint_len(wit.len())
 }
 
-pub(crate) fn witness_to_scriptsig(witness: &[Vec<u8>]) -> ScriptBuf {
+pub(crate) fn witness_to_scriptsig(witness: &[Vec<u8>]) -> Result<ScriptBuf, super::Error> {
     let mut b = script::Builder::new();
-    for wit in witness {
+    for (i, wit) in witness.iter().enumerate() {
         if let Ok(n) = script::read_scriptint(wit) {
             b = b.push_int(n);
         } else {
-            let push = <&PushBytes>::try_from(wit.as_slice())
-                .expect("All pushes in miniscript are <73 bytes");
+            if i != witness.len() - 1 {
+                assert!(wit.len() < 73, "All pushes in miniscript are < 73 bytes");
+            } else {
+                assert!(wit.len() <= MAX_SCRIPT_ELEMENT_SIZE, "P2SH redeem script is <= 520 bytes");
+            }
+            let push = <&PushBytes>::try_from(wit.as_slice()).expect("checked above");
             b = b.push_slice(push)
         }
     }
-    b.into_script()
+    Ok(b.into_script())
 }
 
 // trait for pushing key that depend on context