From d19d3d4cdfff6fa5073ff11dbd42c35682e4c239 Mon Sep 17 00:00:00 2001 From: Pablo Deymonnaz Date: Fri, 13 Mar 2026 19:04:30 -0300 Subject: [PATCH 1/2] 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/2] 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