Prevent receiver panic paths

payjoin/src/core/error.rs

@@ -51,6 +51,8 @@ impl<SessionState: Debug, SessionEvent: Debug> std::fmt::Display
                 Some(session) => write!(f, "Invalid event ({event:?}) for session ({session:?})",),
                 None => write!(f, "Invalid first event ({event:?}) for session",),
             },
+            InvalidEventPayload(event, reason) =>
+                write!(f, "Invalid event payload ({event:?}): {reason}"),
             Expired(time) => write!(f, "Session expired at {time:?}"),
             PersistenceFailure(e) => write!(f, "Persistence failure: {e}"),
         }
@@ -83,6 +85,8 @@ pub(crate) enum InternalReplayError<SessionState, SessionEvent> {
     NoEvents,
     /// Invalid initial event
     InvalidEvent(Box<SessionEvent>, Option<Box<SessionState>>),
+    /// Event payload failed validation
+    InvalidEventPayload(Box<SessionEvent>, String),
     /// Session is expired
     Expired(crate::time::Time),
     /// Application storage error

payjoin/src/core/receive/common/mod.rs

@@ -33,9 +33,12 @@ impl OriginalContext {
     /// Live and replay both route through here so the param sanitization can't diverge.
     pub(super) fn new(original_psbt: Psbt, mut params: Params, owned_vouts: &[usize]) -> Self {
         if let Some((_, additional_fee_output_index)) = params.additional_fee_contribution {
-            // Per BIP78, ignore a fee-contribution index pointing at a receiver output.
+            // Per BIP78, ignore a fee-contribution index that is out of bounds or
+            // pointing at a receiver output.
             // https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#optional-parameters
-            if owned_vouts.contains(&additional_fee_output_index) {
+            if additional_fee_output_index >= original_psbt.unsigned_tx.output.len()
+                || owned_vouts.contains(&additional_fee_output_index)
+            {
                 params.additional_fee_contribution = None;
             }
         }
@@ -1173,4 +1176,41 @@ mod tests {
             .expect("sender output must be present");
         assert_eq!(sender_out.value, sender_value_before - sender_additional_fee);
     }
+
+    // A sender-supplied `additionalfeeoutputindex` past the end of the original outputs
+    // must be dropped at sanitization. Before the bounds check it survived (an
+    // out-of-bounds index is never an owned vout) and `calculate_psbt_with_fee_range`
+    // panicked indexing `original_psbt.unsigned_tx.output` once the receiver
+    // contributed an input and at least one sat of additional fee was due.
+    #[test]
+    fn out_of_bounds_fee_output_index_is_ignored() {
+        let mut original = original_from_test_vector();
+        let output_count = original.psbt.unsigned_tx.output.len();
+        original.params.additional_fee_contribution = Some((Amount::from_sat(182), output_count));
+
+        let wants_inputs = WantsOutputs::new(original, vec![1]).commit_outputs();
+
+        let proposal_psbt = Psbt::from_str(RECEIVER_INPUT_CONTRIBUTION).unwrap();
+        let input = InputPair::new(
+            proposal_psbt.unsigned_tx.input[1].clone(),
+            proposal_psbt.inputs[1].clone(),
+            None,
+        )
+        .unwrap();
+        let wants_fee_range = wants_inputs
+            .contribute_inputs([input])
+            .expect("contribution should succeed")
+            .commit_inputs();
+
+        assert_eq!(
+            wants_fee_range.original.params.additional_fee_contribution, None,
+            "out-of-bounds fee contribution must be dropped at sanitization"
+        );
+        wants_fee_range
+            .calculate_psbt_with_fee_range(
+                Some(FeeRate::from_sat_per_vb_u32(10)),
+                Some(FeeRate::from_sat_per_vb_u32(1000)),
+            )
+            .expect("fee calculation should succeed without the sender contribution");
+    }
 }

payjoin/src/core/receive/v2/mod.rs

@@ -177,18 +177,18 @@ impl ReceiveSession {
 
             (
                 ReceiveSession::OutputsUnknown(state),
-                SessionEvent::IdentifiedReceiverOutputs(wants_outputs),
-            ) => Ok(state.apply_identified_receiver_outputs(wants_outputs)),
+                SessionEvent::IdentifiedReceiverOutputs(owned_vouts),
+            ) => state.apply_identified_receiver_outputs(owned_vouts),
 
             (
                 ReceiveSession::WantsOutputs(state),
                 SessionEvent::CommittedOutputs { outputs, change_vout },
-            ) => Ok(state.apply_committed_outputs(outputs, change_vout)),
+            ) => state.apply_committed_outputs(outputs, change_vout),
 
             (
                 ReceiveSession::WantsInputs(state),
                 SessionEvent::CommittedInputs { receiver_inputs, payjoin_psbt },
-            ) => Ok(state.apply_committed_inputs(receiver_inputs, payjoin_psbt)),
+            ) => state.apply_committed_inputs(receiver_inputs, payjoin_psbt),
 
             (ReceiveSession::WantsFeeRange(state), SessionEvent::AppliedFeeRange(psbt_context)) =>
                 Ok(state.apply_applied_fee_range(psbt_context)),
@@ -268,6 +268,18 @@ impl ReceiveSession {
     }
 }
 
+/// Payload validation failure for an otherwise well-sequenced replay event.
+///
+/// The event log is not trusted to uphold the live path's invariants: it is
+/// deserialized from application storage, and a malformed payload would panic
+/// in later typestates if applied unchecked.
+fn invalid_event_payload(
+    event: SessionEvent,
+    reason: String,
+) -> ReplayError<ReceiveSession, SessionEvent> {
+    InternalReplayError::InvalidEventPayload(Box::new(event), reason).into()
+}
+
 fn pending_fallback_from<S: HasFallbackTx>(r: Receiver<S>) -> ReceiveSession {
     let fallback_tx = r.state.fallback_tx();
     ReceiveSession::PendingFallback(Receiver {
@@ -985,11 +997,20 @@ impl Receiver<OutputsUnknown> {
     pub(crate) fn apply_identified_receiver_outputs(
         self,
         owned_vouts: Vec<usize>,
-    ) -> ReceiveSession {
+    ) -> Result<ReceiveSession, ReplayError<ReceiveSession, SessionEvent>> {
+        let output_count = self.state.original.psbt.unsigned_tx.output.len();
+        if owned_vouts.is_empty() || owned_vouts.iter().any(|&vout| vout >= output_count) {
+            return Err(invalid_event_payload(
+                SessionEvent::IdentifiedReceiverOutputs(owned_vouts),
+                format!(
+                    "owned vouts must be non-empty and within the original PSBT's {output_count} outputs"
+                ),
+            ));
+        }
         let inner = common::WantsOutputs::new(self.state.original, owned_vouts);
         let new_state =
             Receiver { state: WantsOutputs { inner }, session_context: self.session_context };
-        ReceiveSession::WantsOutputs(new_state)
+        Ok(ReceiveSession::WantsOutputs(new_state))
     }
 }
 
@@ -1060,7 +1081,16 @@ impl Receiver<WantsOutputs> {
         self,
         outputs: Vec<TxOut>,
         change_vout: usize,
-    ) -> ReceiveSession {
+    ) -> Result<ReceiveSession, ReplayError<ReceiveSession, SessionEvent>> {
+        let output_count = outputs.len();
+        if change_vout >= output_count {
+            return Err(invalid_event_payload(
+                SessionEvent::CommittedOutputs { outputs, change_vout },
+                format!(
+                    "change vout {change_vout} is out of bounds; {output_count} outputs committed"
+                ),
+            ));
+        }
         let mut payjoin_psbt = self.state.inner.original.original_psbt.clone();
         payjoin_psbt.outputs = vec![Default::default(); outputs.len()];
         payjoin_psbt.unsigned_tx.output = outputs;
@@ -1069,7 +1099,7 @@ impl Receiver<WantsOutputs> {
         let inner = common::WantsInputs { original: self.state.inner.original, proposal };
         let new_state =
             Receiver { state: WantsInputs { inner }, session_context: self.session_context };
-        ReceiveSession::WantsInputs(new_state)
+        Ok(ReceiveSession::WantsInputs(new_state))
     }
 }
 
@@ -1126,14 +1156,23 @@ impl Receiver<WantsInputs> {
         self,
         receiver_inputs: Vec<InputPair>,
         payjoin_psbt: Psbt,
-    ) -> ReceiveSession {
+    ) -> Result<ReceiveSession, ReplayError<ReceiveSession, SessionEvent>> {
         // change_vout is unchanged by contribute_inputs; recover it from the predecessor.
         let change_vout = self.state.inner.proposal.change_vout;
+        let output_count = payjoin_psbt.unsigned_tx.output.len();
+        if change_vout >= output_count {
+            return Err(invalid_event_payload(
+                SessionEvent::CommittedInputs { receiver_inputs, payjoin_psbt },
+                format!(
+                    "change vout {change_vout} is out of bounds; committed PSBT has {output_count} outputs"
+                ),
+            ));
+        }
         let proposal = common::WorkingProposal { payjoin_psbt, change_vout, receiver_inputs };
         let inner = common::WantsFeeRange { original: self.state.inner.original, proposal };
         let new_state =
             Receiver { state: WantsFeeRange { inner }, session_context: self.session_context };
-        ReceiveSession::WantsFeeRange(new_state)
+        Ok(ReceiveSession::WantsFeeRange(new_state))
     }
 }
 

payjoin/src/core/receive/v2/session.rs

@@ -424,6 +424,80 @@ mod tests {
         assert_eq!(replayed, live_wants_outputs, "replayed WantsOutputs must equal the live state");
     }
 
+    // Events up to (excluding) `IdentifiedReceiverOutputs`, from which the malformed
+    // payload tests below diverge.
+    fn events_to_outputs_unknown() -> Vec<SessionEvent> {
+        vec![
+            SessionEvent::Created(SHARED_CONTEXT.clone()),
+            SessionEvent::RetrievedOriginalPayload {
+                original: original_from_test_vector(),
+                reply_key: None,
+            },
+            SessionEvent::CheckedBroadcastSuitability(),
+            SessionEvent::CheckedInputsNotOwned(),
+            SessionEvent::CheckedNoInputsSeenBefore(),
+        ]
+    }
+
+    fn expect_invalid_payload_on_replay(events: Vec<SessionEvent>) {
+        let persister = InMemoryPersister::<SessionEvent>::default();
+        for event in events {
+            persister.save_event(event).expect("In memory persister shouldn't fail");
+        }
+        let err = replay_event_log(&persister).expect_err("replay should reject the payload");
+        assert!(
+            err.to_string().starts_with("Invalid event payload"),
+            "expected an invalid-payload error, got: {err}"
+        );
+    }
+
+    // Regression tests: an event log is only as trustworthy as its storage, so replaying
+    // a malformed payload must produce a ReplayError instead of panicking in a later
+    // typestate (`owned_vouts[0]` or an out-of-bounds `output[change_vout]`).
+    #[test]
+    fn replay_rejects_empty_owned_vouts() {
+        let mut events = events_to_outputs_unknown();
+        events.push(SessionEvent::IdentifiedReceiverOutputs(vec![]));
+        expect_invalid_payload_on_replay(events);
+    }
+
+    #[test]
+    fn replay_rejects_out_of_bounds_owned_vout() {
+        let mut events = events_to_outputs_unknown();
+        let output_count = original_from_test_vector().psbt.unsigned_tx.output.len();
+        events.push(SessionEvent::IdentifiedReceiverOutputs(vec![output_count]));
+        expect_invalid_payload_on_replay(events);
+    }
+
+    #[test]
+    fn replay_rejects_out_of_bounds_change_vout() {
+        let mut events = events_to_outputs_unknown();
+        let outputs = original_from_test_vector().psbt.unsigned_tx.output.clone();
+        let change_vout = outputs.len();
+        events.push(SessionEvent::IdentifiedReceiverOutputs(vec![1]));
+        events.push(SessionEvent::CommittedOutputs { outputs, change_vout });
+        expect_invalid_payload_on_replay(events);
+    }
+
+    #[test]
+    fn replay_rejects_committed_psbt_missing_change_vout() {
+        let mut events = events_to_outputs_unknown();
+        let original = original_from_test_vector();
+        let outputs = original.psbt.unsigned_tx.output.clone();
+        events.push(SessionEvent::IdentifiedReceiverOutputs(vec![1]));
+        events.push(SessionEvent::CommittedOutputs { outputs, change_vout: 1 });
+        // A committed PSBT with too few outputs no longer contains the predecessor's
+        // change vout.
+        let mut truncated = original.psbt.clone();
+        truncated.unsigned_tx.output.truncate(1);
+        truncated.outputs.truncate(1);
+        events.push(SessionEvent::CommittedInputs {
+            receiver_inputs: vec![],
+            payjoin_psbt: truncated,
+        });
+        expect_invalid_payload_on_replay(events);
+    }
+
     #[test]
     fn test_session_event_serialization_roundtrip() {
         let persister = InMemoryPersister::<SessionEvent>::default();