[signer] Add an option to explicitly allow using non-`ALL` sighashes

Instead of blindly using the `sighash_type` set in a psbt input, we
now only sign `SIGHASH_ALL` inputs by default, and require the user to
explicitly opt-in to using other sighashes if they desire to do so.

Fixes #350

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e92c02832b123b125e67139a87526e7053caab64..ddf2b3002b47aa5ee18c359e9285e3800edced91 100644 (file)
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Wallet
+- Added an option that must be explicitly enabled to allow signing using non-`SIGHASH_ALL` sighashes (#350)
+
 ## [v0.7.0] - [v0.6.0]
 
 ### Policy

diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs
index 1b4da36473af5cf9825be5df64d90ef0f2431681..be52cb5a0da9e942f04cda72d8e6105f87033a46 100644 (file)
--- a/src/psbt/mod.rs
+++ b/src/psbt/mod.rs
@@ -63,7 +63,7 @@ mod test {
         psbt.inputs.push(psbt_bip.inputs[0].clone());
         let options = SignOptions {
             trust_witness_utxo: true,
-            assume_height: None,
+            ..Default::default()
         };
         let _ = wallet.sign(&mut psbt, options).unwrap();
     }
@@ -80,7 +80,7 @@ mod test {
         psbt.inputs.push(psbt_bip.inputs[1].clone());
         let options = SignOptions {
             trust_witness_utxo: true,
-            assume_height: None,
+            ..Default::default()
         };
         let _ = wallet.sign(&mut psbt, options).unwrap();
     }
@@ -96,7 +96,7 @@ mod test {
         psbt.global.unsigned_tx.input.push(TxIn::default());
         let options = SignOptions {
             trust_witness_utxo: true,
-            assume_height: None,
+            ..Default::default()
         };
         let _ = wallet.sign(&mut psbt, options).unwrap();
     }

diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs
index 32d5108148bb39587f6bbf937a758159204ce3ee..8f4b85f99fd945850d1153c5d2b8a70f9a0af0f7 100644 (file)
--- a/src/wallet/mod.rs
+++ b/src/wallet/mod.rs
@@ -872,6 +872,17 @@ where
             return Err(Error::Signer(signer::SignerError::MissingNonWitnessUtxo));
         }
 
+        // If the user hasn't explicitly opted-in, refuse to sign the transaction unless every input
+        // is using `SIGHASH_ALL`
+        if !sign_options.allow_all_sighashes
+            && !psbt
+                .inputs
+                .iter()
+                .all(|i| i.sighash_type.is_none() || i.sighash_type == Some(SigHashType::All))
+        {
+            return Err(Error::Signer(signer::SignerError::NonStandardSighash));
+        }
+
         for signer in self
             .signers
             .signers()
@@ -1514,6 +1525,7 @@ pub(crate) mod test {
     use crate::types::KeychainKind;
 
     use super::*;
+    use crate::signer::{SignOptions, SignerError};
     use crate::testutils;
     use crate::wallet::AddressIndex::{LastUnused, New, Peek, Reset};
 
@@ -3670,6 +3682,54 @@ pub(crate) mod test {
         )
     }
 
+    #[test]
+    fn test_sign_nonstandard_sighash() {
+        let sighash = SigHashType::NonePlusAnyoneCanPay;
+
+        let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
+        let addr = wallet.get_address(New).unwrap();
+        let mut builder = wallet.build_tx();
+        builder
+            .set_single_recipient(addr.script_pubkey())
+            .sighash(sighash)
+            .drain_wallet();
+        let (mut psbt, _) = builder.finish().unwrap();
+
+        let result = wallet.sign(&mut psbt, Default::default());
+        assert!(
+            result.is_err(),
+            "Signing should have failed because the TX uses non-standard sighashes"
+        );
+        assert!(
+            matches!(
+                result.unwrap_err(),
+                Error::Signer(SignerError::NonStandardSighash)
+            ),
+            "Signing failed with the wrong error type"
+        );
+
+        // try again after opting-in
+        let result = wallet.sign(
+            &mut psbt,
+            SignOptions {
+                allow_all_sighashes: true,
+                ..Default::default()
+            },
+        );
+        assert!(result.is_ok(), "Signing should have worked");
+        assert!(
+            result.unwrap(),
+            "Should finalize the input since we can produce signatures"
+        );
+
+        let extracted = psbt.extract_tx();
+        assert_eq!(
+            *extracted.input[0].witness[0].last().unwrap(),
+            sighash.as_u32() as u8,
+            "The signature should have been made with the right sighash"
+        );
+    }
+
     #[test]
     fn test_unused_address() {
         let db = MemoryDatabase::new();

diff --git a/src/wallet/signer.rs b/src/wallet/signer.rs
index 76ffc3d7912506f60a3f6b83a71fc4724c89de6a..cf901c6a042caee01f1d0e72a16ee08475708039 100644 (file)
--- a/src/wallet/signer.rs
+++ b/src/wallet/signer.rs
@@ -147,6 +147,12 @@ pub enum SignerError {
     MissingWitnessScript,
     /// The fingerprint and derivation path are missing from the psbt input
     MissingHdKeypath,
+    /// The psbt contains a non-`SIGHASH_ALL` sighash in one of its input and the user hasn't
+    /// explicitly allowed them
+    ///
+    /// To enable signing transactions with non-standard sighashes set
+    /// [`SignOptions::allow_all_sighashes`] to `true`.
+    NonStandardSighash,
 }
 
 impl fmt::Display for SignerError {
@@ -465,6 +471,12 @@ pub struct SignOptions {
     /// timelock height has already been reached. This option allows overriding the "current height" to let the
     /// wallet use timelocks in the future to spend a coin.
     pub assume_height: Option<u32>,
+
+    /// Whether the signer should use the `sighash_type` set in the PSBT when signing, no matter
+    /// what its value is
+    ///
+    /// Defaults to `false` which will only allow signing using `SIGHASH_ALL`.
+    pub allow_all_sighashes: bool,
 }
 
 impl Default for SignOptions {
@@ -472,6 +484,7 @@ impl Default for SignOptions {
         SignOptions {
             trust_witness_utxo: false,
             assume_height: None,
+            allow_all_sighashes: false,
         }
     }
 }