From 759d0e81a422b5663b519f3701e37f2b5fb53c9e Mon Sep 17 00:00:00 2001 From: spacebear Date: Sat, 10 Aug 2024 16:39:57 -0400 Subject: [PATCH 1/2] Fix sender output checks `check_outputs` was incorrectly iterating over the proposal outputs, effectively skipping all output checks. This commit fixes that and ensures the offical test vectors pass. --- payjoin/src/send/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index 8b4644e9..b1c27751 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -800,7 +800,8 @@ impl ContextV1 { } fn check_outputs(&self, proposal: &Psbt) -> InternalResult { - let mut original_outputs = proposal.unsigned_tx.output.iter().enumerate().peekable(); + let mut original_outputs = + self.original_psbt.unsigned_tx.output.iter().enumerate().peekable(); let mut total_value = bitcoin::Amount::ZERO; let mut contributed_fee = bitcoin::Amount::ZERO; let mut total_weight = Weight::ZERO; @@ -821,7 +822,7 @@ impl ContextV1 { { if proposed_txout.value < original_output.value { contributed_fee = original_output.value - proposed_txout.value; - ensure!(contributed_fee < max_fee_contrib, FeeContributionExceedsMaximum); + ensure!(contributed_fee <= max_fee_contrib, FeeContributionExceedsMaximum); //The remaining fee checks are done in the caller } original_outputs.next(); @@ -1058,7 +1059,7 @@ mod test { let ctx = super::ContextV1 { original_psbt, disable_output_substitution: false, - fee_contribution: None, + fee_contribution: Some((bitcoin::Amount::from_sat(182), 0)), min_fee_rate: FeeRate::ZERO, payee, input_type: InputType::SegWitV0 { ty: SegWitV0Type::Pubkey, nested: true }, From 183f85b1d34311712b2d5749cdf1526e3ce11975 Mon Sep 17 00:00:00 2001 From: spacebear Date: Mon, 12 Aug 2024 12:04:17 -0400 Subject: [PATCH 2/2] Test attempt to steal sender change This adds a test that attempts to steal change from the sender output and add that amount to the receiver output, and is expected to panic. --- payjoin/src/send/mod.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/payjoin/src/send/mod.rs b/payjoin/src/send/mod.rs index b1c27751..5a19f6f7 100644 --- a/payjoin/src/send/mod.rs +++ b/payjoin/src/send/mod.rs @@ -1085,6 +1085,27 @@ mod test { ctx.process_proposal(proposal).unwrap(); } + #[test] + #[should_panic] + fn test_receiver_steals_sender_change() { + let original_psbt = Psbt::from_str(ORIGINAL_PSBT).unwrap(); + eprintln!("original: {:#?}", original_psbt); + let ctx = create_v1_context(); + let mut proposal = Psbt::from_str(PAYJOIN_PROPOSAL).unwrap(); + eprintln!("proposal: {:#?}", proposal); + for output in proposal.outputs_mut() { + output.bip32_derivation.clear(); + } + for input in proposal.inputs_mut() { + input.bip32_derivation.clear(); + } + proposal.inputs_mut()[0].witness_utxo = None; + // Steal 0.5 BTC from the sender output and add it to the receiver output + proposal.unsigned_tx.output[0].value -= bitcoin::Amount::from_btc(0.5).unwrap(); + proposal.unsigned_tx.output[1].value += bitcoin::Amount::from_btc(0.5).unwrap(); + ctx.process_proposal(proposal).unwrap(); + } + #[test] #[cfg(feature = "v2")] fn req_ctx_ser_de_roundtrip() {