From 3337b7a030795547077e1920120bf0f82463bf8e Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 23 Jun 2025 20:13:55 +0000 Subject: [PATCH 1/3] psbt: introduce IncorrectNonWitnessUtxo error variant Putting this in its own commit so that the fix and the test can live in separate commits which reviewers can swap. --- bitcoin/src/psbt/error.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/bitcoin/src/psbt/error.rs b/bitcoin/src/psbt/error.rs index 58338ab69..6095c85ff 100644 --- a/bitcoin/src/psbt/error.rs +++ b/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 From 74f22a0a67f95ec6dc993cc550b3122d4527bd42 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 23 Jun 2025 20:13:55 +0000 Subject: [PATCH 2/3] psbt: validate that non_witness_utxo txids match the input txids This requires updating a couple serde test vectors -- we are tightening validation so that some existing serde-serialized PSBTs no longer deserialize. I believe this is the correct thing to do. --- bitcoin/src/psbt/mod.rs | 1 + bitcoin/src/psbt/serialize.rs | 16 ++++++++++++++-- bitcoin/tests/data/serde/psbt_base64.json | 2 +- bitcoin/tests/data/serde/psbt_bincode | Bin 810 -> 810 bytes bitcoin/tests/serde.rs | 1 + 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index e78328fee..e55480b88 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -1667,6 +1667,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 diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index 542c9798f..468afae2d 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -103,8 +103,20 @@ impl Psbt { let mut inputs: Vec = 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 diff --git a/bitcoin/tests/data/serde/psbt_base64.json b/bitcoin/tests/data/serde/psbt_base64.json index 6bb318f10..d05155177 100644 --- a/bitcoin/tests/data/serde/psbt_base64.json +++ b/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==" \ No newline at end of file +"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==" \ No newline at end of file diff --git a/bitcoin/tests/data/serde/psbt_bincode b/bitcoin/tests/data/serde/psbt_bincode index 35d580f4b0ddd26395b7974739e0cd48ad82fbe4..0423835c0ee0459e4560ca54f4c3583e246b7664 100644 GIT binary patch delta 43 zcmV+`0M!4g2C4>-6(Bhjk(O4C4if;%nmR|b+1YmcoPn|Tck-mV$H(N2>n5=^^#Y#~ B7196z delta 13 VcmZ3*wu)_n=)@N)8!bOF0RSbZ1%Lnm diff --git a/bitcoin/tests/serde.rs b/bitcoin/tests/serde.rs index 3d2b4a28a..0a4f4bbb1 100644 --- a/bitcoin/tests/serde.rs +++ b/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 From 53b93321c87ed57680e0ca49a0691c4deb7edde7 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 23 Jun 2025 20:04:54 +0000 Subject: [PATCH 3/3] psbt: add test vector where non-witness UTXO TXID does not match input txid --- bitcoin/src/psbt/mod.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index e55480b88..4fe91060e 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -2113,6 +2113,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