Merge rust-bitcoin/rust-bitcoin#4637: psbt: check that non-witness UTXOs' txids match the input txid

53b93321c8 psbt: add test vector where non-witness UTXO TXID does not match input txid (Andrew Poelstra)
74f22a0a67 psbt: validate that non_witness_utxo txids match the input txids (Andrew Poelstra)
3337b7a030 psbt: introduce IncorrectNonWitnessUtxo error variant (Andrew Poelstra)

Pull request description:

  Fixes #4617


ACKs for top commit:
  tcharding:
    ACK 53b93321c8


Tree-SHA512: bf841708493fe38a37c7448cc02a26061ea66fd3c7acdbf9df4bbdfe07d8611075dceed20813de2b7cd63864ad80a9f66eaf8b869249c3a2028e1be25b48c5ae

bitcoin/src/psbt/error.rs

@@ -80,6 +80,16 @@ pub enum Error {
     NegativeFee,
     /// Integer overflow in fee calculation
     FeeOverflow,
+    /// Non-witness UTXO (which is a complete transaction) has [`crate::Txid`] that
+    /// does not match the transaction input.
+    IncorrectNonWitnessUtxo {
+        /// The index of the input in question.
+        index: usize,
+        /// The outpoint of the input, as it appears in the unsigned transaction.
+        input_outpoint: crate::OutPoint,
+        /// The ['crate::Txid`] of the non-witness UTXO.
+        non_witness_utxo_txid: crate::Txid,
+    },
     /// Parsing error indicating invalid public keys
     InvalidPublicKey(crate::crypto::key::FromSliceError),
     /// Parsing error indicating invalid secp256k1 public keys
@@ -154,6 +164,13 @@ impl fmt::Display for Error {
                 write_err!(f, "error parsing bitcoin consensus encoded object"; e),
             NegativeFee => f.write_str("PSBT has a negative fee which is not allowed"),
             FeeOverflow => f.write_str("integer overflow in fee calculation"),
+            IncorrectNonWitnessUtxo { index, input_outpoint, non_witness_utxo_txid } => {
+                write!(
+                    f,
+                    "non-witness utxo txid is {}, which does not match input {}'s outpoint {}",
+                    non_witness_utxo_txid, index, input_outpoint
+                )
+            }
             InvalidPublicKey(ref e) => write_err!(f, "invalid public key"; e),
             InvalidSecp256k1PublicKey(ref e) => write_err!(f, "invalid secp256k1 public key"; e),
             InvalidXOnlyPublicKey => f.write_str("invalid xonly public key"),
@@ -200,6 +217,7 @@ impl std::error::Error for Error {
             | CombineInconsistentKeySources(_)
             | NegativeFee
             | FeeOverflow
+            | IncorrectNonWitnessUtxo { .. }
             | InvalidPublicKey(_)
             | InvalidSecp256k1PublicKey(_)
             | InvalidXOnlyPublicKey

bitcoin/src/psbt/mod.rs

@@ -1666,6 +1666,7 @@ mod tests {
             },
             unsigned_tx: {
                 let mut unsigned = tx.clone();
+                unsigned.input[0].previous_output.txid = tx.compute_txid();
                 unsigned.input[0].script_sig = ScriptBuf::new();
                 unsigned.input[0].witness = Witness::default();
                 unsigned
@@ -2111,6 +2112,28 @@ mod tests {
         }
     }
 
+    #[test]
+    fn invalid_vector_4617() {
+        let err = hex_psbt("70736274ff01007374ff0103010000000000000000002e2873007374ff0107736205000000000000000000000000000000000006060005feffffff74ff01000a000000000000002cc760008530b38dac0100030500000074ff01070100000000000000000000000000c0316888e006000600050000736274ff00d90001007374ff41030100000000000a0a06002e2873007374ff01070100000000000000000000000000000000ff0000060600050000736274ff01000a0080000000000024c7600005193b1e400700030500000074ff0107010000000000a9c7df3f07000570ed62c76004c3ca95c5f90200010742420a0a000000000000").unwrap_err();
+        match err {
+            Error::IncorrectNonWitnessUtxo { index: 0, input_outpoint, non_witness_utxo_txid } => {
+                assert_eq!(
+                    input_outpoint,
+                    "00000000000000000000000562730701ff74730073282e000000000000000000:0"
+                        .parse()
+                        .unwrap(),
+                );
+                assert_eq!(
+                    non_witness_utxo_txid,
+                    "9ed45fd3f73b038649bee6e763dbd70868745c48a0d2b0299f42c68f957995f4"
+                        .parse()
+                        .unwrap(),
+                );
+            }
+            _ => panic!("expected output hash mismatch error, got {}", err),
+        }
+    }
+
     #[test]
     fn serialize_and_deserialize_preimage_psbt() {
         // create a sha preimage map

bitcoin/src/psbt/serialize.rs

@@ -103,8 +103,20 @@ impl Psbt {
 
             let mut inputs: Vec<Input> = Vec::with_capacity(inputs_len);
 
-            for _ in 0..inputs_len {
-                inputs.push(Input::decode(r)?);
+            for i in 0..inputs_len {
+                let input = Input::decode(r)?;
+                if let Some(ref tx) = input.non_witness_utxo {
+                    let input_outpoint = global.unsigned_tx.input[i].previous_output;
+                    let txid = tx.compute_txid();
+                    if txid != input_outpoint.txid {
+                        return Err(Error::IncorrectNonWitnessUtxo {
+                            index: i,
+                            input_outpoint,
+                            non_witness_utxo_txid: txid,
+                        });
+                    }
+                }
+                inputs.push(input);
             }
 
             inputs

bitcoin/tests/data/serde/psbt_base64.json

@@ -1 +1 @@
-"cHNidP8BAFMBAAAAAYmjxx6rTSDgNxu7pMxpj6KVyUY6+i45f4UzzLYvlWflAQAAAAD/////AXL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHAAAAAE8BBIiyHgAAAAAAAAAAAIc9/4HAL1JWI/0f5RZ+rDpVoEnePTFLtC7iJ//tN9UIAzmjYBMwFZfa70H75ZOgLMUT0LVVJ+wt8QUOLo/0nIXCDN6tvu8AAACAAQAAABD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFAAEAjwEAAAAAAQGJo8ceq00g4Dcbu6TMaY+ilclGOvouOX+FM8y2L5Vn5QEAAAAXFgAUvhjRUqmwEgOdrz2n3k9TNJ7suYX/////AXL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHASED0uFWdJQbrUqZY3LLh+GFbTZSYG2YVi/jnF6efkE/IQUAAAAAAQEgcv74TiwAAAAXqRQzlyW6Ie/WKsdTqbzQZ9bHpqOdBYciAgM5iA3JI5S3NV49BDn6KDwx3nWQgS6gEcQkXAZ0poXog0cwRAIgT2fir7dhQtRPrliiSV0zo0GdqibNDbjQTzRStjKJrA8CIBB2Kp+2fpTMXK2QJvbcmf9/Bw9CeNMPvH0Mhp3TjH/nAQEDBIMAAAABBAFRIgYDOYgNySOUtzVePQQ5+ig8Md51kIEuoBHEJFwGdKaF6IMM3q2+7wAAAIABAAAAAQgGAgIBAwEFFQoYn3yLGjhv/o7tkbODDHp7zR53jAIBAiELoShx/uIQ+4YZKR6uoZRYHL0lMeSyN1nSJfaAaSP2MiICAQIVDBXMSeGRy8Ug2RlEYApct3r2qjKRAgECIQ12pWrO2RXSUT3NhMLDeLLoqlzWMrW3HKLyrFsOOmSb2wIBAhD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFACICAzmIDckjlLc1Xj0EOfooPDHedZCBLqARxCRcBnSmheiDDN6tvu8AAACAAQAAABD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFAA=="
+"cHNidP8BAFMBAAAAATkUkZZWjQ4TAMqaOkez2dl2+5yBsfd38qS6x8fkjesmAQAAAAD/////AXL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHAAAAAE8BBIiyHgAAAAAAAAAAAIc9/4HAL1JWI/0f5RZ+rDpVoEnePTFLtC7iJ//tN9UIAzmjYBMwFZfa70H75ZOgLMUT0LVVJ+wt8QUOLo/0nIXCDN6tvu8AAACAAQAAABD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFAAEAjwEAAAAAAQGJo8ceq00g4Dcbu6TMaY+ilclGOvouOX+FM8y2L5Vn5QEAAAAXFgAUvhjRUqmwEgOdrz2n3k9TNJ7suYX/////AXL++E4sAAAAF6kUM5cluiHv1irHU6m80GfWx6ajnQWHASED0uFWdJQbrUqZY3LLh+GFbTZSYG2YVi/jnF6efkE/IQUAAAAAAQEgcv74TiwAAAAXqRQzlyW6Ie/WKsdTqbzQZ9bHpqOdBYciAgM5iA3JI5S3NV49BDn6KDwx3nWQgS6gEcQkXAZ0poXog0cwRAIgT2fir7dhQtRPrliiSV0zo0GdqibNDbjQTzRStjKJrA8CIBB2Kp+2fpTMXK2QJvbcmf9/Bw9CeNMPvH0Mhp3TjH/nAQEDBIMAAAABBAFRIgYDOYgNySOUtzVePQQ5+ig8Md51kIEuoBHEJFwGdKaF6IMM3q2+7wAAAIABAAAAAQgGAgIBAwEFFQoYn3yLGjhv/o7tkbODDHp7zR53jAIBAiELoShx/uIQ+4YZKR6uoZRYHL0lMeSyN1nSJfaAaSP2MiICAQIVDBXMSeGRy8Ug2RlEYApct3r2qjKRAgECIQ12pWrO2RXSUT3NhMLDeLLoqlzWMrW3HKLyrFsOOmSb2wIBAhD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFACICAzmIDckjlLc1Xj0EOfooPDHedZCBLqARxCRcBnSmheiDDN6tvu8AAACAAQAAABD8BXByZWZ4KnRlc3Rfa2V5AwUGBwMJAAEDAwQFAA=="

bitcoin/tests/serde.rs

@@ -285,6 +285,7 @@ fn serde_regression_psbt() {
         },
         unsigned_tx: {
             let mut unsigned = tx.clone();
+            unsigned.input[0].previous_output.txid = tx.compute_txid();
             unsigned.input[0].script_sig = ScriptBuf::new();
             unsigned.input[0].witness = Witness::default();
             unsigned