Merge pull request #233 from darosior/finalizer_check_sig

Explicitly error on non-standard sighash types

Author: apoelstra

src/interpreter/error.rs

@@ -12,7 +12,7 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use bitcoin::hashes::hash160;
+use bitcoin::hashes::{hash160, hex::ToHex};
 use bitcoin::{self, secp256k1};
 use std::{error, fmt};
 
@@ -39,6 +39,8 @@ pub enum Error {
     InsufficientSignaturesMultiSig,
     /// Signature failed to verify
     InvalidSignature(bitcoin::PublicKey),
+    /// Last byte of this signature isn't a standard sighash type
+    NonStandardSigHash(Vec<u8>),
     /// Miniscript error
     Miniscript(::Error),
     /// MultiSig requires 1 extra zero element apart from the `k` signatures
@@ -128,6 +130,13 @@ impl fmt::Display for Error {
             Error::IncorrectWScriptHash => f.write_str("witness script did not match scriptpubkey"),
             Error::InsufficientSignaturesMultiSig => f.write_str("Insufficient signatures for CMS"),
             Error::InvalidSignature(pk) => write!(f, "bad signature with pk {}", pk),
+            Error::NonStandardSigHash(ref sig) => {
+                write!(
+                    f,
+                    "Non standard sighash type for signature '{}'",
+                    sig.to_hex()
+                )
+            }
             Error::NonEmptyWitness => f.write_str("legacy spend had nonempty witness"),
             Error::NonEmptyScriptSig => f.write_str("segwit spend had nonempty scriptsig"),
             Error::Miniscript(ref e) => write!(f, "parse error: {}", e),

src/interpreter/mod.rs

@@ -757,7 +757,8 @@ where
     F: FnOnce(&bitcoin::PublicKey, BitcoinSig) -> bool,
 {
     if let Some((sighash_byte, sig)) = sigser.split_last() {
-        let sighashtype = bitcoin::SigHashType::from_u32(*sighash_byte as u32);
+        let sighashtype = bitcoin::SigHashType::from_u32_standard(*sighash_byte as u32)
+            .map_err(|_| Error::NonStandardSigHash([sig, &[*sighash_byte]].concat().to_vec()))?;
         let sig = secp256k1::Signature::from_der(sig)?;
         if verify_sig(pk, (sig, sighashtype)) {
             Ok(sig)

src/miniscript/satisfy.rs

@@ -30,7 +30,6 @@ use miniscript::limits::{
     HEIGHT_TIME_THRESHOLD, SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG,
 };
 use util::witness_size;
-use Error;
 use Miniscript;
 use ScriptContext;
 use Terminal;
@@ -43,9 +42,10 @@ pub type Preimage32 = [u8; 32];
 /// Helper function to create BitcoinSig from Rawsig
 /// Useful for downstream when implementing Satisfier.
 /// Returns underlying secp if the Signature is not of correct format
-pub fn bitcoinsig_from_rawsig(rawsig: &[u8]) -> Result<BitcoinSig, Error> {
+pub fn bitcoinsig_from_rawsig(rawsig: &[u8]) -> Result<BitcoinSig, ::interpreter::Error> {
     let (flag, sig) = rawsig.split_last().unwrap();
-    let flag = bitcoin::SigHashType::from_u32(*flag as u32);
+    let flag = bitcoin::SigHashType::from_u32_standard(*flag as u32)
+        .map_err(|_| ::interpreter::Error::NonStandardSigHash([sig, &[*flag]].concat().to_vec()))?;
     let sig = secp256k1::Signature::from_der(sig)?;
     Ok((sig, flag))
 }

src/psbt/finalizer.rs

@@ -262,7 +262,14 @@ pub fn finalize<C: secp256k1::Verification>(
                 ));
             }
             let (flag, sig) = rawsig.split_last().unwrap();
-            let flag = bitcoin::SigHashType::from_u32(*flag as u32);
+            let flag = bitcoin::SigHashType::from_u32_standard(*flag as u32).map_err(|_| {
+                super::Error::InputError(
+                    InputError::Interpreter(interpreter::Error::NonStandardSigHash(
+                        [sig, &[*flag]].concat().to_vec(),
+                    )),
+                    n,
+                )
+            })?;
             if target != flag {
                 return Err(Error::InputError(
                     InputError::WrongSigHashFlag {