## Summary The `filter_unsynced_commitments` function does not handle empty input and will panic if called with an empty vector of commitments. This panic is reachable via the public `batchProver_createCircuitInput` RPC method, allowing an unauthenticated remote attacker to crash the batch prover node, causing a Denial of Service. ## Finding Description The `filter_unsynced_commitments` function in `batch-prover/src/prover.rs` contains code that unconditionally expects the input `commitments` vector to be non-empty: ```rust // batch-prover/src/prover.rs let l2_end_block_number = commitments .last() .expect("Commitments must not be empty"); // <-- Panics if `commitments` is empty ``` An external user can trigger this panic through the following steps: 1. An attacker calls the `batchProver_createCircuitInput` RPC method with a range of indices (e.g., very high numbers) that are guaranteed not to exist in the node's database. 2. The RPC handler in `batch-prover/src/rpc.rs` queries the database and correctly receives an empty `Vec`. 3. This empty vector is sent in a `ProverRequest::CreateInput` message to the main `Prover` task. 4. The `Prover` task calls `create_partitions`, which in turn calls `filter_unsynced_commitments` with the empty vector. 5. The `.expect()` call is triggered on a `None` value, causing a panic that crashes the `Prover`'s async task and terminates the entire node. This vulnerability breaks the **availability** guarantee of the batch prover. A critical component of the rollup infrastructure can be taken offline by a simple, remote request. ## Impact Explanation **Impact: Critical** The impact is critical because this vulnerability allows for a complete and repeatable Denial of Service on the batch prover node. * **Service Takedown:** A successful exploit immediately crashes the prover. * **Liveness Halt:** A crashed prover cannot generate or submit new batch proofs, halting a critical function of the rollup and preventing the L2 state from being settled on the DA layer. * **Low Attacker Cost:** The attack is trivial and cheap to execute, requiring only a single RPC call. * **Unauthenticated Vector:** The RPC endpoint is a public-facing interface, making the attack accessible to any malicious actor on the network. ## Likelihood Explanation The likelihood of exploitation is high. The attack vector is a public, unauthenticated RPC endpoint. The condition required to trigger the panic (an empty set of commitments for a given range) is easy for an attacker to create. The result is deterministic; an empty vector will always cause a panic. ## Proof of Concept 1. Set up and run a Citrea `batch-prover` node. 2. Using an RPC client like `curl`, send the following request to the batch prover's RPC endpoint. The high index range ensures no commitments are found. ```json { "jsonrpc": "2.0", "method": "batchProver_createCircuitInput", "params": [999999, 1000000, "Normal"], "id": 1 } ``` 3. **Observe:** The `batch-prover` node will immediately panic and shut down. The logs will show a panic message originating from the `.expect("Commitments must not be empty")` line in `filter_unsynced_commitments`. ## Recommendation The function must be made robust against empty inputs. The recommended fix involves two parts: adding a guard clause to handle the empty case and refactoring the filtering functions to be more idiomatic and performant by operating on a mutable reference. ```rust commitments: &'a mut Vec, mode: PartitionMode, ) -> anyhow::Result>> { self.filter_unsynced_commitments(commitments)?; if commitments.is_empty() { warn!("L2 blocks not synced up to any of the pending commitments yet"); return Ok(Vec::new()); } info!("Got {} synced commitment(s)", commitments.len()); self.filter_commitments_with_index_gap(commitments)?; if commitments.is_empty() { warn!("Previous commitment of pending commitment list is not known yet"); return Ok(Vec::new()); } info!("Got {} provable commitment(s)", commitments.len()); let partitions = self.partition_commitments(commitments, mode)?; info!("Partitioned commitments into {} parts", partitions.len()); /// A vector of sequencer commitments that are synced to the L2 blocks. fn filter_unsynced_commitments( &mut self, commitments: &mut Vec, ) -> anyhow::Result<()> { if commitments.is_empty() { return Ok(()); } let head_l2_height = self.ledger_db.get_head_l2_block_height()?.unwrap_or(0); let l2_end_block_number = commitments .last() .expect("Commitments must not be empty") if l2_end_block_number <= head_l2_height { // short circuit for fully synced case self.sync_target_l2_height = None; return Ok(()); } // find first commitment position that is not synced let sync_target_l2_height = commitments[unsynced_pos].l2_end_block_number; self.sync_target_l2_height = Some(sync_target_l2_height); let unsynced_count = commitments.len() - unsynced_pos; commitments.truncate(unsynced_pos); warn!( "Only synced up to height {}, ignoring {} commitments, next sync target height is {}", head_l2_height, unsynced_count, sync_target_l2_height ); Ok(()) } /// Filters out the commitments that has index gaps, hence, can't be proven. Expects commitments to be in ascending order by index. /// E.g. commitments = [3, 4, 6], and commitment 2 is known, outputs [3, 4] fn filter_commitments_with_index_gap( &self, commitments: &mut Vec, ) -> anyhow::Result<()> { if commitments.is_empty() { return Ok(()); } let first_index = commitments[0].index; let mut prev_index = if first_index == 1 { 0 } else { match self.ledger_db.get_commitment_by_index(first_index - 1)? { Some(_) => first_index - 1, // prev commitment not found, truncate the whole vector None => { commitments.clear(); return Ok(()); } } }; if let Some(gap_pos) = commitments.windows(2).position(|w| w[1].index != w[0].index + 1) { commitments.truncate(gap_pos + 1); } Ok(()) } /// Partition the commitments into provable chunks. ```

edoscoba

Public reviews