From d19d3d4cdfff6fa5073ff11dbd42c35682e4c239 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 13 Mar 2026 19:04:30 -0300 Subject: [PATCH 1/3] Guard against out-of-bounds indexing on attacker-controlled data in attestation processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defensive fixes in process_attestations / try_finalize: 1. aggregation_bits OOB (finding #6): A malicious attestation could carry aggregation_bits longer than the validator set, causing votes[validator_id] to panic on out-of-bounds access. Now filter bits >= validator_count before indexing. 2. root_to_slot missing key (finding #7): try_finalize used direct HashMap index root_to_slot[root] which panics if a justification root is absent from historical_block_hashes (e.g. carried over from a prior finalization window). Replaced with .get() — missing roots are conservatively retained. 3. justifications[root] (finding #8): Reviewed and confirmed safe — the roots come from justifications.keys() so the key is always present. No change needed. --- crates/blockchain/state_transition/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index 2deece0..c23b484 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -284,11 +284,13 @@ fn process_attestations( .entry(target.root) .or_insert_with(|| std::iter::repeat_n(false, validator_count).collect()); // Mark that each validator in this aggregation has voted for the target. + // Skip bits beyond validator_count — a malicious attestation could have + // aggregation_bits longer than the actual validator set, causing OOB panic. for (validator_id, _) in attestation .aggregation_bits .iter() .enumerate() - .filter(|(_, voted)| *voted) + .filter(|(id, voted)| *voted && *id < validator_count) { votes[validator_id] = true; } @@ -416,10 +418,14 @@ fn try_finalize( let delta = (state.latest_finalized.slot - old_finalized_slot) as usize; justified_slots_ops::shift_window(&mut state.justified_slots, delta); - // Prune justifications whose roots only appear at now-finalized slots + // Prune justifications whose roots only appear at now-finalized slots. + // Use .get() instead of direct index — a root may be absent from root_to_slot + // if it was never in historical_block_hashes (e.g. carried over from a previous + // finalization window). Missing roots are conservatively retained. justifications.retain(|root, _| { - let slot = root_to_slot[root]; - slot > state.latest_finalized.slot + root_to_slot + .get(root) + .is_none_or(|&slot| slot > state.latest_finalized.slot) }); } From 364eaa80a8fae5ae0ab72144b91b9cc7a2d0d66f Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 13 Mar 2026 19:08:23 -0300 Subject: [PATCH 2/3] Use get_mut for aggregation_bits bounds check instead of filter predicate The bounds guard is now at the point of access rather than in the iterator filter. get_mut returns None for out-of-bounds indices, making the safety invariant self-contained in a single expression. --- crates/blockchain/state_transition/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index c23b484..e992b70 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -284,15 +284,17 @@ fn process_attestations( .entry(target.root) .or_insert_with(|| std::iter::repeat_n(false, validator_count).collect()); // Mark that each validator in this aggregation has voted for the target. - // Skip bits beyond validator_count — a malicious attestation could have - // aggregation_bits longer than the actual validator set, causing OOB panic. + // Use get_mut to safely skip bits beyond the validator set — a malicious + // attestation could carry aggregation_bits longer than validator_count. for (validator_id, _) in attestation .aggregation_bits .iter() .enumerate() - .filter(|(id, voted)| *voted && *id < validator_count) + .filter(|(_, voted)| *voted) { - votes[validator_id] = true; + if let Some(vote) = votes.get_mut(validator_id) { + *vote = true; + } } // Check whether the vote count crosses the supermajority threshold From 8dcbe3e6d559ef530db68b335161ba4c0114904a Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Mon, 16 Mar 2026 17:35:03 -0300 Subject: [PATCH 3/3] Align OOB aggregation_bits and justification pruning fixes with spec behavior Reject entire attestations when aggregation_bits exceeds validator count (matching spec crash, Zeam reject, Lantern reject) instead of silently skipping individual OOB bits. Prune justification roots missing from root_to_slot (their slot is at or below finalized_slot) instead of conservatively retaining them forever. --- crates/blockchain/state_transition/src/lib.rs | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/crates/blockchain/state_transition/src/lib.rs b/crates/blockchain/state_transition/src/lib.rs index e992b70..4d6fff9 100644 --- a/crates/blockchain/state_transition/src/lib.rs +++ b/crates/blockchain/state_transition/src/lib.rs @@ -7,7 +7,7 @@ use ethlambda_types::{ primitives::{H256, ssz::TreeHash}, state::{HISTORICAL_ROOTS_LIMIT, JustificationValidators, State}, }; -use tracing::info; +use tracing::{info, warn}; mod justified_slots_ops; pub mod metrics; @@ -283,18 +283,23 @@ fn process_attestations( let votes = justifications .entry(target.root) .or_insert_with(|| std::iter::repeat_n(false, validator_count).collect()); + // Reject attestations with aggregation_bits longer than the validator set. + // The spec would crash (IndexError) on OOB access; Zeam and Lantern reject. + if attestation.aggregation_bits.len() > validator_count { + warn!( + bits_len = attestation.aggregation_bits.len(), + validator_count, "Skipping attestation: aggregation_bits exceeds validator count" + ); + continue; + } // Mark that each validator in this aggregation has voted for the target. - // Use get_mut to safely skip bits beyond the validator set — a malicious - // attestation could carry aggregation_bits longer than validator_count. for (validator_id, _) in attestation .aggregation_bits .iter() .enumerate() .filter(|(_, voted)| *voted) { - if let Some(vote) = votes.get_mut(validator_id) { - *vote = true; - } + votes[validator_id] = true; } // Check whether the vote count crosses the supermajority threshold @@ -420,14 +425,19 @@ fn try_finalize( let delta = (state.latest_finalized.slot - old_finalized_slot) as usize; justified_slots_ops::shift_window(&mut state.justified_slots, delta); - // Prune justifications whose roots only appear at now-finalized slots. - // Use .get() instead of direct index — a root may be absent from root_to_slot - // if it was never in historical_block_hashes (e.g. carried over from a previous - // finalization window). Missing roots are conservatively retained. - justifications.retain(|root, _| { - root_to_slot - .get(root) - .is_none_or(|&slot| slot > state.latest_finalized.slot) + // Prune justifications whose roots are at or below the finalized slot. + // The spec asserts all roots must be in root_to_slot (state.py L560). + // A missing root means its slot <= finalized_slot, so prune it. + justifications.retain(|root, _| match root_to_slot.get(root) { + Some(&slot) => slot > state.latest_finalized.slot, + None => { + warn!( + root = %ShortRoot(&root.0), + finalized_slot = state.latest_finalized.slot, + "Justification root missing from root_to_slot, pruning" + ); + false + } }); }