fix: reject unchallenged response chunks

Author: Zk2u

crates/cac/protocol/src/evaluator/stf.rs

@@ -6,7 +6,7 @@ use mosaic_cac_types::{
     ChallengeResponseMsgChunk, ChallengeResponseMsgHeader, CommitMsgChunk, CommitMsgHeader,
     CompletedSignatures, DepositAdaptors, DepositId, GarblingTableCommitment, HeapArray, Index,
     OpenedGarblingTableCommitments, PubKey, ReservedSetupInputShares, Seed, SetupInputs,
-    TableTransferReceiptMsg, TableTransferRequestMsg, WideLabelWirePolynomialCommitments,
+    TableTransferReceiptMsg, WideLabelWirePolynomialCommitments,
     WideLabelZerothPolynomialCoefficients, WithdrawalAdaptors, WithdrawalAdaptorsChunk,
     WithdrawalInputs, state_machine::evaluator::*,
 };
@@ -582,7 +582,7 @@ async fn handle_commit_msg_header<S: StateMut>(
     match &mut state.step {
         Step::WaitingForCommit { header, chunks, .. } => {
             if *header {
-                warn!("evaluator received duplicate commit header, ack and ignore");
+                debug!("evaluator received duplicate commit header, ack and ignore");
                 return Ok(());
             }
 
@@ -637,7 +637,7 @@ async fn handle_commit_msg_header<S: StateMut>(
             post_handle_commit_msg(state, artifact_store, actions).await
         }
         step if step.phase() > StepPhase::WaitingForCommit => {
-            warn!("evaluator received commit header after completion, ack and ignore");
+            debug!("evaluator received commit header after completion, ack and ignore");
             Ok(())
         }
         _ => Err(SMError::UnexpectedInput),
@@ -659,7 +659,7 @@ async fn handle_commit_msg_chunk<S: StateMut>(
                 }
                 Some(true) => {
                     // already seen chunk
-                    warn!(%chunk_idx, "evaluator received duplicate commit chunk, ack and ignore");
+                    debug!(%chunk_idx, "evaluator received duplicate commit chunk, ack and ignore");
                     return Ok(());
                 }
                 None => {
@@ -699,7 +699,7 @@ async fn handle_commit_msg_chunk<S: StateMut>(
             post_handle_commit_msg(root_state, state, actions).await
         }
         step if step.phase() > StepPhase::WaitingForCommit => {
-            warn!("evaluator received commit chunk after completion, ack and ignore");
+            debug!("evaluator received commit chunk after completion, ack and ignore");
             Ok(())
         }
         _ => Err(SMError::UnexpectedInput),
@@ -744,7 +744,7 @@ async fn handle_recv_challenge_response_header<S: StateMut>(
             remaining_chunks,
         } => {
             if *header {
-                warn!("evaluator received duplicate challenge response header, ack and ignore");
+                debug!("evaluator received duplicate challenge response header, ack and ignore");
                 return Ok(());
             }
 
@@ -789,7 +789,7 @@ async fn handle_recv_challenge_response_header<S: StateMut>(
             post_handle_challenge_response(root_state, state, actions).await
         }
         step if step.phase() > StepPhase::WaitingForChallengeResponse => {
-            warn!("evaluator received challenge response header after completion, ack and ignore");
+            debug!("evaluator received challenge response header after completion, ack and ignore");
             Ok(())
         }
         _ => Err(SMError::UnexpectedInput),
@@ -807,6 +807,18 @@ async fn handle_recv_challenge_response_msg<S: StateMut>(
             header,
             remaining_chunks,
         } => {
+            let challenge_idxs = state
+                .get_challenge_indices()
+                .await
+                .require("expected challenge indices")?;
+
+            if !challenge_idxs
+                .iter()
+                .any(|idx| idx.get() == response_msg_chunk.circuit_index as usize)
+            {
+                return Err(SMError::InvalidInputData);
+            }
+
             let chunk_idx = (response_msg_chunk.circuit_index as usize)
                 .checked_sub(1)
                 .unwrap();
@@ -815,8 +827,11 @@ async fn handle_recv_challenge_response_msg<S: StateMut>(
                     // expected chunk
                 }
                 Some(false) => {
-                    // already seen chunk
-                    warn!(%chunk_idx, "evaluator received duplicate commit chunk, ack and ignore");
+                    // already seen expected chunk
+                    debug!(
+                        %chunk_idx,
+                        "evaluator received duplicate challenge response chunk, ack and ignore"
+                    );
                     return Ok(());
                 }
                 None => {
@@ -847,7 +862,7 @@ async fn handle_recv_challenge_response_msg<S: StateMut>(
             post_handle_challenge_response(root_state, state, actions).await
         }
         step if step.phase() > StepPhase::WaitingForChallengeResponse => {
-            warn!("evaluator received challenge response chunk after completion, ack and ignore");
+            debug!("evaluator received challenge response chunk after completion, ack and ignore");
             Ok(())
         }
         _ => Err(SMError::UnexpectedInput),

crates/cac/protocol/src/evaluator/tests.rs

@@ -396,6 +396,10 @@ async fn duplicate_challenge_response_chunk_is_ack_and_ignore() {
     // Mark first challenge index's chunk as already received.
     let first_challenge_idx = challenge_indices[0].get() - 1;
     remaining[first_challenge_idx] = false;
+    state
+        .put_challenge_indices(&challenge_indices)
+        .await
+        .unwrap();
     state
         .put_root_state(&EvaluatorState {
             config: None,
@@ -422,6 +426,41 @@ async fn duplicate_challenge_response_chunk_is_ack_and_ignore() {
     assert!(actions.is_empty(), "should produce no actions");
 }
 
+#[tokio::test]
+async fn unchallenged_in_range_challenge_response_chunk_is_invalid() {
+    let mut state = StoredEvaluatorState::default();
+    let challenge_indices = ChallengeIndices::new(|i| Index::new((i * 2) + 1).unwrap());
+    let remaining = get_remaining_challenge_response_chunks(&challenge_indices);
+    state
+        .put_challenge_indices(&challenge_indices)
+        .await
+        .unwrap();
+    state
+        .put_root_state(&EvaluatorState {
+            config: None,
+            step: Step::WaitingForChallengeResponse {
+                header: false,
+                remaining_chunks: remaining,
+            },
+        })
+        .await
+        .unwrap();
+
+    let mut actions = Vec::new();
+    let result = handle_event(
+        &mut state,
+        Input::RecvChallengeResponseMsgChunk(dummy_challenge_response_chunk(2)),
+        &mut actions,
+    )
+    .await;
+
+    assert!(
+        matches!(result, Err(crate::error::SMError::InvalidInputData)),
+        "unchallenged in-range chunk must be rejected, got: {result:?}"
+    );
+    assert!(actions.is_empty(), "should produce no actions");
+}
+
 #[tokio::test]
 async fn challenge_response_chunk_after_verifying_is_ack_and_ignore() {
     let mut state = StoredEvaluatorState::default();

crates/cac/protocol/src/garbler/stf.rs

@@ -143,7 +143,7 @@ pub(crate) async fn handle_event<S: StateMut>(
             }
             // ack on all steps after WaitingForChallenge
             step if step.phase() > StepPhase::WaitingForChallenge => {
-                warn!("garbler received challenge after completion, ack and ignore");
+                debug!("garbler received challenge after completion, ack and ignore");
                 return Ok(());
             }
             _ => return Err(SMError::unexpected_input()),
@@ -175,7 +175,7 @@ pub(crate) async fn handle_event<S: StateMut>(
                 emit(actions, Action::TransferGarblingTable(*seed));
             }
             step if step.phase() > StepPhase::TransferringGarblingTables => {
-                warn!("garbler received table transfer request after completion, ack and ignore");
+                debug!("garbler received table transfer request after completion, ack and ignore");
                 return Ok(());
             }
             _ => return Err(SMError::unexpected_input()),
@@ -212,7 +212,7 @@ pub(crate) async fn handle_event<S: StateMut>(
                 }
             }
             step if step.phase() > StepPhase::TransferringGarblingTables => {
-                warn!("garbler received table transfer receipt after completion, ack and ignore");
+                debug!("garbler received table transfer receipt after completion, ack and ignore");
                 return Ok(());
             }
             _ => return Err(SMError::unexpected_input()),
@@ -400,7 +400,7 @@ pub(crate) async fn handle_action_result<S: StateMut>(
                     }
                 }
                 step if step.phase() > StepPhase::SendingCommit => {
-                    warn!("garbler received commit header ack after completion, ignore");
+                    debug!("garbler received commit header ack after completion, ignore");
                 }
                 _ => return Err(SMError::unexpected_input()),
             }
@@ -434,7 +434,7 @@ pub(crate) async fn handle_action_result<S: StateMut>(
                     }
                 }
                 step if step.phase() > StepPhase::SendingCommit => {
-                    warn!("garbler received commit chunk ack after completion, ignore");
+                    debug!("garbler received commit chunk ack after completion, ignore");
                 }
                 _ => return Err(SMError::unexpected_input()),
             }
@@ -525,7 +525,7 @@ pub(crate) async fn handle_action_result<S: StateMut>(
                     // Informational only. We mark a table as transferred once we get corresponding
                     // [`TableTransferReceiptMsg`](mosaic_cac_types::TableTransferReceiptMsg) from
                     // evaluator.
-                    info!(%commitment, "garbling table transferred")
+                    debug!(%commitment, "garbling table transferred")
                 }
                 _ => return Err(SMError::unexpected_input()),
             }
@@ -784,14 +784,14 @@ async fn handle_recv_deposit_adaptor_msg_chunk<S: StateMut>(
                 let DepositStep::WaitingForAdaptors { chunks } = &mut deposit_state.step else {
                     // WaitingForAdaptors is first step of deposit state machine. All other steps
                     // are after it.
-                    warn!("garbler received adaptor chunk after completion, ack and ignore");
+                    debug!("garbler received adaptor chunk after completion, ack and ignore");
                     return Ok(());
                 };
 
                 let chunk_idx = adaptor_msg_chunk.chunk_index as usize;
 
                 if chunks[chunk_idx] {
-                    warn!("garbler received duplicate adaptor chunk, ack and ignore");
+                    debug!("garbler received duplicate adaptor chunk, ack and ignore");
                     return Ok(());
                 }
 
@@ -824,7 +824,7 @@ async fn handle_recv_deposit_adaptor_msg_chunk<S: StateMut>(
             }
         }
         step if step.phase() > StepPhase::SetupComplete => {
-            warn!("garbler received adaptor chunk after completion, ack and ignore");
+            debug!("garbler received adaptor chunk after completion, ack and ignore");
             return Ok(());
         }