zaino-state has a Non-Finalized State Reorg — No Cycle Detection or Depth Limit

Summary

### Summary `NonFinalizedState::handle_reorg` is a recursive, unbounded async function that traverses parent blocks until it finds a common ancestor on the main chain. It has **no recursion depth limit** and **no cycle detection**. A malicious or buggy validator can serve a block whose `previous_block_hash` points back to itself (or forms a cycle with other blocks), causing `handle_reorg` to infinite-loop, consuming 100% CPU and never making sync progress. Additionally, `update()` contains an `.expect("empty snapshot impossible")` that panics if the non-finalized snapshot becomes empty after trimming finalized blocks. ### Details **Location:** `packages/zaino-state/src/chain_index/non_finalised_state.rs:443-489` ```rust async fn handle_reorg( &self, working_snapshot: &mut NonfinalizedBlockCacheSnapshot, block: &impl Block, ) -> Result<IndexedBlock, SyncError> { let prev_block = match working_snapshot .get_block_by_hash_bytes_in_serialized_order(block.prev_hash_bytes_serialized_order()) .cloned() { Some(prev_block) => { if !working_snapshot .heights_to_hashes .values() .any(|hash| hash == prev_block.hash()) { Box::pin(self.handle_reorg(working_snapshot, &prev_block)).await? // <-- LINE 459 } else { prev_block } } None => { let prev_block = self .source .get_block(HashOrHeight::Hash( zebra_chain::block::Hash::from_bytes_in_serialized_order( block.prev_hash_bytes_serialized_order(), ), )) .await .map_err(|e| { ... })? .ok_or(SyncError::ValidatorConnectionError(...))?; Box::pin(self.handle_reorg(working_snapshot, &*prev_block)).await? // <-- LINE 483 } }; let indexed_block = block.to_indexed_block(&prev_block, self).await?; working_snapshot.add_block_new_chaintip(indexed_block.clone()); Ok(indexed_block) } ``` **Infinite loop via self-referencing block:** 1. A compromised validator serves a block `B` where `B.prev_hash == B.hash`. 2. `handle_reorg` is called with `B`. 3. `get_block_by_hash_bytes_in_serialized_order(B.prev_hash)` finds `B` itself in `working_snapshot.blocks`. 4. Check: is `B.hash` in `working_snapshot.heights_to_hashes`? If `B` is a new chaintip not yet on the main chain, **no**. 5. Recurse with `prev_block` = `B` (the exact same block). 6. This repeats forever. The async recursion builds a new `Box::pin` future each iteration, consuming heap memory and CPU. **Stack exhaustion via deep reorg:** A deep reorg of >1000 blocks would recurse >1000 times. Each async recursion creates a new `Box::pin` future on the heap. While this won't exhaust the native stack immediately, it will allocate unbounded heap memory and CPU time, effectively DoS-ing the sync task. **`.expect("empty snapshot impossible")` panic:** **Location:** `packages/zaino-state/src/chain_index/non_finalised_state.rs:543-548` ```rust new_snapshot.remove_finalized_blocks(finalized_height); let best_block = &new_snapshot .blocks .values() .max_by_key(|block| block.chainwork()) .cloned() .expect("empty snapshot impossible"); // <-- LINE 548 ``` If `finalized_height` is greater than or equal to all blocks in `new_snapshot.blocks`, `remove_finalized_blocks` retains only blocks at or above that height. If none exist, `new_snapshot.blocks` becomes empty. The `.expect()` then panics. While the comment claims this is "impossible," defensive programming dictates it is reachable under corruption or edge-case sync conditions. ### PoC 1. Run a regtest. 2. Serve a block where `header.previous_block_hash == block.hash()`. 3. Zaino's `NonFinalizedState::sync` enters `handle_reorg` and infinite-loops. 4. Sync never completes. CPU usage pegs to 100%. No new blocks are served to clients. ### Fix 1. **Add an explicit recursion depth limit** (e.g., max 1000 iterations) and return `SyncError::ReorgFailure` if exceeded: ```rust const MAX_REORG_DEPTH: usize = 1000; ``` 2. **Track visited hashes** in a `HashSet<BlockHash>` during traversal to detect cycles and abort with an error. 3. **Replace `.expect("empty snapshot impossible")`** with a proper `Err(UpdateError::DatabaseHole)` or similar error return. ### Additional Attack Vectors - **Deep reorg DoS:** A miner with significant hash power (or a compromised validator) triggers a deep reorg. Zaino spends excessive CPU and memory in `handle_reorg`, starving the async runtime and stalling response serving. - **Fork-choice manipulation:** By serving cyclic or very deep sidechains, an attacker can keep Zaino stuck in reorg handling indefinitely, preventing it from ever serving the real best chain.