diff --git a/CHANGELOG.md b/CHANGELOG.md index e92c0283..ddf2b300 100644 --- 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 1b4da364..be52cb5a 100644 --- 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 32d51081..8f4b85f9 100644 --- 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 76ffc3d7..cf901c6a 100644 --- 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, + + /// 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, } } }