From 2a53932c7587073e3ac5a47e1afaea0b3eb31591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 6 Jul 2026 14:25:45 -0300 Subject: [PATCH] fix(fork-choice): bound a block's slot before the state transition Port leanSpec #1182. on_block had no upper bound on a block's slot, so one validly-signed block on a reachable parent could drive the empty-slot walk over an unbounded range. Add two guards at the untrusted-input boundary, before the (expensive) signature verification so a crafted block is rejected cheaply: - parent-gap cap: reject block.slot - parent.slot > HISTORICAL_ROOTS_LIMIT, bounding the walk directly and independent of the clock. - clock horizon: reject block.slot > current_slot + 1, keeping far-future blocks out of the store (whole-slot margin so an intended early block still imports). ethlambda's process_slots already jumps straight to the target slot (no per-slot loop) and process_block_header already caps the historical-roots allocation, so this is defense-in-depth plus the cheap early rejection the spec places here. --- crates/blockchain/src/store.rs | 111 ++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/crates/blockchain/src/store.rs b/crates/blockchain/src/store.rs index 4b12bb3f..4e22e7a5 100644 --- a/crates/blockchain/src/store.rs +++ b/crates/blockchain/src/store.rs @@ -12,7 +12,7 @@ use ethlambda_types::{ checkpoint::Checkpoint, primitives::{H256, HashTreeRoot as _}, signature::{ValidatorPublicKey, ValidatorSignature}, - state::State, + state::{HISTORICAL_ROOTS_LIMIT, State}, }; use tracing::{info, trace, warn}; @@ -535,6 +535,31 @@ fn on_block_core( slot, })?; + // Bound the block's slot before the state transition runs (leanSpec #1182). + // + // The transition advances the state one slot at a time from the parent up to + // `block.slot`, so a block far beyond its parent, or far in the future, would + // drive that walk unboundedly. Both guards live here at the untrusted-input + // boundary and run before the expensive signature verification, so a crafted + // block is rejected cheaply. + let slot_gap = slot.saturating_sub(parent_state.slot); + if slot_gap > HISTORICAL_ROOTS_LIMIT as u64 { + return Err(StoreError::BlockSlotGapTooLarge { + gap: slot_gap, + max: HISTORICAL_ROOTS_LIMIT as u64, + }); + } + // Horizon is the current slot plus one whole slot of margin, so an intended + // early block still imports (mirrors the attestation future-slot guard, but + // with a whole-slot rather than one-interval margin). + let current_slot = store.time() / INTERVALS_PER_SLOT; + if slot > current_slot + 1 { + return Err(StoreError::BlockTooFarInFuture { + block_slot: slot, + current_slot, + }); + } + // Each unique AttestationData must appear at most once per block. let attestations = &signed_block.message.body.attestations; let mut seen = HashSet::with_capacity(attestations.len()); @@ -949,6 +974,12 @@ pub enum StoreError { #[error("Block contains {count} distinct AttestationData entries; maximum is {max}")] TooManyAttestationData { count: usize, max: usize }, + + #[error("Block slot gap {gap} beyond parent exceeds historical roots limit {max}")] + BlockSlotGapTooLarge { gap: u64, max: u64 }, + + #[error("Block slot {block_slot} is beyond the future horizon (current slot: {current_slot})")] + BlockTooFarInFuture { block_slot: u64, current_slot: u64 }, } /// Full verification of a signed block's merged multi-message aggregate proof. @@ -1500,4 +1531,82 @@ mod tests { "fully canonical vote must validate" ); } + + /// leanSpec #1182: a block whose slot is more than one slot past the store + /// clock is rejected before the state transition (and before signature + /// verification), keeping far-future blocks out of the store. + #[test] + fn on_block_rejects_block_too_far_in_future() { + use ethlambda_storage::backend::InMemoryBackend; + use std::sync::Arc; + + let genesis_state = State::from_genesis(1000, vec![]); + let backend = Arc::new(InMemoryBackend::new()); + let mut store = Store::from_anchor_state(backend, genesis_state); + store.set_time(0).expect("set_time should succeed"); + + // current_slot = 0, so the horizon is slot 1; a slot-2 block overshoots it. + let block = Block { + slot: 2, + proposer_index: 0, + parent_root: store.head(), + state_root: H256::ZERO, + body: BlockBody::default(), + }; + let signed_block = SignedBlock { + message: block, + proof: MultiMessageAggregate::default(), + }; + + let result = on_block_without_verification(&mut store, signed_block); + assert!( + matches!( + result, + Err(StoreError::BlockTooFarInFuture { + block_slot: 2, + current_slot: 0, + }) + ), + "Expected BlockTooFarInFuture, got: {result:?}" + ); + } + + /// leanSpec #1182: a block whose slot runs more than HISTORICAL_ROOTS_LIMIT + /// beyond its parent is rejected up front, so the transition never walks the + /// empty-slot loop over an unbounded range. The gap guard runs before the + /// future-horizon guard, so it fires even with the clock at genesis. + #[test] + fn on_block_rejects_block_slot_gap_too_large() { + use ethlambda_storage::backend::InMemoryBackend; + use std::sync::Arc; + + let genesis_state = State::from_genesis(1000, vec![]); + let backend = Arc::new(InMemoryBackend::new()); + let mut store = Store::from_anchor_state(backend, genesis_state); + store.set_time(0).expect("set_time should succeed"); + + // Parent (genesis) sits at slot 0, so a slot one past the limit overshoots. + let gap_slot = HISTORICAL_ROOTS_LIMIT as u64 + 1; + let block = Block { + slot: gap_slot, + proposer_index: 0, + parent_root: store.head(), + state_root: H256::ZERO, + body: BlockBody::default(), + }; + let signed_block = SignedBlock { + message: block, + proof: MultiMessageAggregate::default(), + }; + + let result = on_block_without_verification(&mut store, signed_block); + assert!( + matches!( + result, + Err(StoreError::BlockSlotGapTooLarge { gap, max }) + if gap == gap_slot && max == HISTORICAL_ROOTS_LIMIT as u64 + ), + "Expected BlockSlotGapTooLarge, got: {result:?}" + ); + } }