diff --git a/src/error.rs b/src/error.rs index bcb36ef..20e8e54 100644 --- a/src/error.rs +++ b/src/error.rs @@ -56,6 +56,10 @@ pub enum Error { SigHashTypeInvalid { source: bitcoin::sighash::InvalidSighashTypeError, }, + #[snafu(display("Non-standard sighash type: {source}"))] + SigHashTypeNonStandard { + source: bitcoin::sighash::NonStandardSighashTypeError, + }, #[snafu(display("Unsupported sighash type `{sighash_type}`"))] SigHashTypeUnsupported { sighash_type: String }, #[snafu(display("Not key path spend"))] @@ -68,10 +72,6 @@ pub enum Error { PublicKeyMismatch, #[snafu(display("At least one private key is required"))] NoPrivateKeys, - #[snafu(display("Non-standard sighash type: {source}"))] - SigHashTypeNonStandard { - source: bitcoin::sighash::NonStandardSighashTypeError, - }, #[snafu(display("Signer's public key not present in multisig script"))] UnknownSigner, #[snafu(display("Duplicate private key provided"))] diff --git a/src/lib.rs b/src/lib.rs index 1e2fe11..67dcfc1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -274,6 +274,22 @@ mod tests { ); } + #[test] + fn verify_p2wpkh_signature_shorter_than_71_bytes() { + // ECDSA signatures are not fixed length: this signature's DER encoding is 69 + // bytes (r serializes to 31 bytes), i.e. 70 bytes with the sighash flag, below + // the usual 71/72. It is valid and `sign_simple` produces it, so `verify` + // must accept it. Regenerate with: + // sign::sign_simple_encoded(SEGWIT_ADDRESS, "probe-266", WIF_PRIVATE_KEY) + assert!( + verify::verify_simple_encoded( + SEGWIT_ADDRESS, + "probe-266", + "AkYwQwIgdHvqo7c5BbXCr0O5xWkT1qoihgF5oaKXoFlzuegR+ZICHxoQGPcMKj+iUTymjR5tC+uN7arZcZHUv7BMyf6rwJoBIQLH8SADGWRClD2FiOAa7oQEI8xU/BUhUmo7hcKwy9WIcg==" + ).is_ok() + ); + } + #[test] fn simple_sign_p2wpkh() { assert_eq!( diff --git a/src/verify.rs b/src/verify.rs index 9ee2188..6d0d5b7 100644 --- a/src/verify.rs +++ b/src/verify.rs @@ -184,23 +184,23 @@ fn verify_full_p2wpkh( return Err(Error::ToSignInvalid); } + if encoded_signature.is_empty() { + return Err(Error::SignatureLength { + length: 0, + encoded_signature, + }); + } + let signature_length = encoded_signature.len(); - let (signature, sighash_type) = match signature_length { - 71 | 72 => ( - bitcoin::secp256k1::ecdsa::Signature::from_der( - &encoded_signature.as_slice()[..signature_length - 1], - ) - .context(error::SignatureInvalid)?, - EcdsaSighashType::from_consensus(encoded_signature[signature_length - 1] as u32), - ), - _ => { - return Err(Error::SignatureLength { - length: encoded_signature.len(), - encoded_signature, - }) - } - }; + let signature = bitcoin::secp256k1::ecdsa::Signature::from_der( + &encoded_signature.as_slice()[..signature_length - 1], + ) + .context(error::SignatureInvalid)?; + + let sighash_type = + EcdsaSighashType::from_standard(encoded_signature[signature_length - 1] as u32) + .context(error::SigHashTypeNonStandard)?; if !(sighash_type == EcdsaSighashType::All) { return Err(Error::SigHashTypeUnsupported {