zaino-state has a Non-Finalized State Reorg — No Cycle Detection or Depth LimitGHSA-3whf-vgf2-9w6g

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

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

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:

   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.