Skip to content

Commit

Permalink
Error parsing on non-standard sighash bytes
Browse files Browse the repository at this point in the history
This moves from silently accepting non-standard sighash types with u32
to explicitly Erroring in such a case.

We should never be managing signatures with such sighash types anyways,
as they would not relay on today's Bitcoin network.

Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
  • Loading branch information
darosior committed Jun 14, 2021
1 parent f550b9b commit c289fdf
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 6 deletions.
11 changes: 10 additions & 1 deletion src/interpreter/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 3 additions & 1 deletion src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,9 @@ 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)
Expand Down
6 changes: 3 additions & 3 deletions src/miniscript/satisfy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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))
}
Expand Down
9 changes: 8 additions & 1 deletion src/psbt/finalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit c289fdf

Please sign in to comment.