fix(psbt): Validate previous non-witness UTXO txid in PSBT signing process

rust/chains/tw_bitcoin/src/modules/psbt_request/mod.rs

@@ -9,6 +9,8 @@ use tw_proto::BitcoinV2::Proto;
 use tw_utxo::context::UtxoContext;
 use tw_utxo::transaction::unsigned_transaction::UnsignedTransaction;
 
+pub use bitcoin::psbt::Psbt;
+
 pub mod output_psbt;
 pub mod standard_psbt_request_handler;
 pub mod utxo_psbt;

rust/chains/tw_bitcoin/src/modules/psbt_request/utxo_psbt.rs

@@ -8,7 +8,7 @@ use secp256k1::ThirtyTwoByteHash;
 use tw_coin_entry::error::prelude::*;
 use tw_hash::H256;
 use tw_utxo::script::Script;
-use tw_utxo::sighash::SighashType;
+use tw_utxo::sighash::{SighashBase, SighashType};
 use tw_utxo::transaction::standard_transaction::builder::UtxoBuilder;
 use tw_utxo::transaction::standard_transaction::TransactionInput;
 use tw_utxo::transaction::UtxoToSign;
@@ -34,10 +34,10 @@ impl<'a> UtxoPsbt<'a> {
     }
 
     pub fn build(self) -> SigningResult<(TransactionInput, UtxoToSign)> {
-        if let Some(ref non_witness_utxo) = self.utxo_psbt.non_witness_utxo {
-            self.build_non_witness_utxo(non_witness_utxo)
-        } else if let Some(ref witness_utxo) = self.utxo_psbt.witness_utxo {
+        if let Some(ref witness_utxo) = self.utxo_psbt.witness_utxo {
             self.build_witness_utxo(witness_utxo)
+        } else if let Some(ref non_witness_utxo) = self.utxo_psbt.non_witness_utxo {
+            self.build_non_witness_utxo(non_witness_utxo)
         } else {
             SigningError::err(SigningErrorType::Error_invalid_params)
                 .context("Neither 'witness_utxo' nor 'non_witness_utxo' are set in the PSBT")
@@ -57,6 +57,14 @@ impl<'a> UtxoPsbt<'a> {
                 format!("'Psbt::non_witness_utxo' does not contain '{prev_out_idx}' output")
             })?;
 
+        let expected_txid = non_witness_utxo.txid();
+        let actual_txid = self.utxo.previous_output.txid;
+        if actual_txid != expected_txid {
+            return SigningError::err(SigningErrorType::Error_invalid_utxo).context(format!(
+                "Txid mismatch between PSBT input and non-witness UTXO: PSBT references '{actual_txid}', but non-witness UTXO has '{expected_txid}'",
+            ));
+        }
+
         let script = Script::from(prev_out.script_pubkey.to_bytes());
         let builder = self.prepare_builder(prev_out.value)?;
 
@@ -113,6 +121,11 @@ impl<'a> UtxoPsbt<'a> {
             None => SighashType::default(),
         };
 
+        if sighash_ty.base_type() != SighashBase::All || sighash_ty.anyone_can_pay() {
+            return SigningError::err(SigningErrorType::Error_not_supported)
+                .context("Only SIGHASH_ALL is supported for PSBT inputs");
+        }
+
         let amount = amount
             .try_into()
             .tw_err(SigningErrorType::Error_invalid_utxo_amount)
@@ -130,3 +143,22 @@ impl<'a> UtxoPsbt<'a> {
         !self.utxo_psbt.tap_scripts.is_empty()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use bitcoin::consensus::Decodable;
+    use std::io::Cursor;
+    use tw_encoding::hex::DecodeHex;
+
+    /// This test is to verify that the `bitcoin` crate correctly computes the `txid` for a transaction with both witness and non-witness inputs.
+    #[test]
+    fn test_mixed_witness_and_non_witness_txid() {
+        let mut cursor = Cursor::new("01000000000102fff7f7881a8099afa6940d42d1e7f6362bec38171ea3edf433541db4e4ad969f0000000049483045022100fd8591c3611a07b55f509ec850534c7a9c49713c9b8fa0e844ea06c2e65e19d702205e3806676192e790bc93dd4c28e937c4bf97b15f189158ba1a30d7ecff5ee75503ffffffffef51e1b804cc89d182d279655c3aa89e815b1b309fe287d9b2b55d57b90ec68a0100000000ffffffff02b0bf0314000000001976a914769bdff96a02f9135a1d19b749db6a78fe07dc9088ac4bf00405000000001976a9149e089b6889e032d46e3b915a3392edfd616fb1c488ac00024730440220096d20c7e92f991c2bf38dc28118feb34019ae74ec1c17179b28cb041de7517402204594f46a911f24bdc7109ca192e6860ebf2f3a0087579b3c128d5ce0cd5ed4680321025476c2e83188368da1ff3e292e7acafcdb3566bb0ad253f62fc70f07aeee635700000000".decode_hex().unwrap());
+        let tx = bitcoin::Transaction::consensus_decode(&mut cursor).unwrap();
+        let txid = tx.txid();
+        assert_eq!(
+            txid.to_string(),
+            "68c08a357a16b163983f7338185dc8befdf3e301e648b1cceca26a3fd33fefbd"
+        );
+    }
+}

rust/chains/tw_zcash/src/modules/pczt_request/utxo_pczt.rs

@@ -10,7 +10,7 @@ use tw_coin_entry::error::prelude::{
 };
 use tw_hash::H256;
 use tw_utxo::script::Script;
-use tw_utxo::sighash::SighashType;
+use tw_utxo::sighash::{SighashBase, SighashType};
 use tw_utxo::transaction::standard_transaction::builder::UtxoBuilder;
 use tw_utxo::transaction::standard_transaction::TransactionInput;
 use tw_utxo::transaction::transaction_parts::Amount;
@@ -48,6 +48,11 @@ impl<'a> UtxoPczt<'a> {
         let sighash_ty = SighashType::from_u32(self.utxo.sighash_type as u32)
             .context("Invalid sighash type in PCZT UTXO")?;
 
+        if sighash_ty.base_type() != SighashBase::All || sighash_ty.anyone_can_pay() {
+            return SigningError::err(SigningErrorType::Error_not_supported)
+                .context("Only SIGHASH_ALL is supported for PSBT inputs");
+        }
+
         let builder = UtxoBuilder::default()
             .prev_txid(prevout_hash)
             .prev_index(prevout_index)

rust/tw_tests/Cargo.toml

@@ -18,6 +18,7 @@ tw_number = { path = "../tw_number" }
 tw_proto = { path = "../tw_proto" }
 wallet-core-rs = { path = "../wallet_core_rs" }
 # Chain specific:
+tw_bitcoin = { path = "../chains/tw_bitcoin" }
 tw_cosmos_sdk = { path = "../tw_cosmos_sdk", features = ["test-utils"] }
 tw_solana = { path = "../chains/tw_solana" }
 tw_ton = { path = "../chains/tw_ton" }

rust/tw_tests/tests/chains/bitcoin/bitcoin_sign/psbt.rs

@@ -4,9 +4,11 @@
 
 use crate::chains::common::bitcoin::psbt_sign::{BitcoinPsbtSignHelper, Expected};
 use crate::chains::common::bitcoin::transaction_psbt;
+use tw_any_coin::test_utils::sign_utils::AnySignerHelper;
 use tw_coin_registry::coin_type::CoinType;
-use tw_encoding::hex::DecodeHex;
+use tw_encoding::hex::{DecodeHex, ToHex};
 use tw_proto::BitcoinV2::Proto;
+use tw_proto::Common::Proto::SigningError;
 
 #[test]
 fn test_bitcoin_sign_psbt_thorchain_swap_witness() {
@@ -60,3 +62,42 @@ fn test_bitcoin_sign_psbt_thorchain_swap_non_witness() {
             fee: 2662,
         });
 }
+
+#[test]
+fn test_bitcoin_sign_psbt_non_witness_tampered_output_value() {
+    // 1CKZYtNxAQnTbygz6vyhBYnwx4NvcxURMB
+    let private_key = "7a87cb2c9fa56f7a63dfc50659dca260473cb6bb0fd4d8a2beeaf5357d41de95"
+        .decode_hex()
+        .unwrap();
+
+    let original_psbt_bytes = "70736274ff01008202000000015c37bcf049b7e62dd5bfd707e0998ce86163b786e3cd45db2336cb794a8d8aa10000000000ffffffff03f82a000000000000160014bf5a13a26791a5db6406304a46952e264c2b28910000000000000000056a032b3a6291950000000000001976a9147c2c0ac72afbde13ecf52fca54368e7883b538b188ac000000000001007e0200000002714916920be4dbc87cbb8697ca9b1420d6b1e47e7d732e2d2e0e7a935087788d0000000000ffffffff326c951cd9b3dc382e2d6be88796b65d7bac90406a5f72660171ac826e414a630200000000ffffffff01efca0000000000001976a9147c2c0ac72afbde13ecf52fca54368e7883b538b188ac0000000000000000"
+        .decode_hex()
+        .unwrap();
+    let mut original_psbt =
+        tw_bitcoin::modules::psbt_request::Psbt::deserialize(&original_psbt_bytes).unwrap();
+    let prev_output = &mut original_psbt.inputs[0]
+        .non_witness_utxo
+        .as_mut()
+        .unwrap()
+        .output[0];
+    assert_eq!(prev_output.value, 51_951);
+    // Change the amount to forge a transaction with an invalid input amount, that leads to an invalid txid.
+    prev_output.value = 1000;
+    let wrong_psbt = original_psbt.serialize().to_hex();
+
+    let input = Proto::SigningInput {
+        private_keys: vec![private_key.into()],
+        transaction: transaction_psbt(&wrong_psbt),
+        ..Proto::SigningInput::default()
+    };
+
+    let mut signer = AnySignerHelper::<Proto::SigningOutput>::default();
+    let output = signer.sign(CoinType::Bitcoin, input);
+
+    assert_eq!(
+        output.error,
+        SigningError::Error_invalid_utxo,
+        "Expected Error_invalid_utxo. Error message: {}",
+        output.error_message
+    );
+}