Skip to content

Commit 8382476

Browse files
tcoratgerclaude
andauthored
fix: bound attestation slot against head and store clock (leanEthereum#1020)
* fix: bound attestation slot against head and store clock The gossip attestation validator fed the unbounded wire field attestation_data.slot straight into the interval conversion, which multiplies it by the intervals-per-slot count and wraps the product in a range-checked unsigned 64-bit constructor. A crafted gossip attestation with a near-2**64 slot raised a pydantic validation error that escaped the gossip handler's rejection-only catch and crashed the node, a one-message remote denial of service. The root cause is that the slot was never tied to the head it claims. This change adds two checks after the topology checks: - The vote's slot must not precede the slot of the head block it claims to have seen, surfaced as a new ATTESTATION_SLOT_BEFORE_HEAD rejection. This anchors the wire slot from below by a known block and enforces the 3SF attestation semantics where the slot records when the head was seen. - The future-time gate now compares in slot units with plain-int arithmetic before constructing any interval, so a near-ceiling slot is rejected cleanly instead of overflowing. The integer comparison is equivalent to the previous interval comparison. Adds two negative consensus vectors: one for a slot at the unsigned 64-bit ceiling rejected without overflow, and one for a slot preceding its head. All 115 fork-choice vectors still fill green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: align attestation slot-bound comments with doc rules Split multi-clause sentences onto their own lines and replace code-identifier-style names (intervals_per_slot, horizon) in the head-consistency and time-check comments with plain English, per the documentation rules. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: simplify attestation slot-bound comments Cut the head-consistency and time-check comments to their essential reason (two lines each) and shorten the new rejection-reason docstring. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f01af40 commit 8382476

3 files changed

Lines changed: 136 additions & 3 deletions

File tree

src/lean_spec/spec/forks/lstar/errors.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ class RejectionReason(StrEnum):
7777
ATTESTATION_TOO_FAR_IN_FUTURE = "ATTESTATION_TOO_FAR_IN_FUTURE"
7878
"""The attestation slot is beyond the store's acceptance horizon."""
7979

80+
ATTESTATION_SLOT_BEFORE_HEAD = "ATTESTATION_SLOT_BEFORE_HEAD"
81+
"""The attestation slot precedes its head block's slot."""
82+
8083
VALIDATOR_NOT_IN_STATE = "VALIDATOR_NOT_IN_STATE"
8184
"""The referenced validator does not exist in the state registry."""
8285

src/lean_spec/spec/forks/lstar/fork_choice.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from lean_spec.spec.forks.lstar._base import LstarSpecBase, LstarStore
1010
from lean_spec.spec.forks.lstar.config import (
1111
GOSSIP_DISPARITY_INTERVALS,
12+
INTERVALS_PER_SLOT,
1213
MAX_ATTESTATIONS_DATA,
1314
)
1415
from lean_spec.spec.forks.lstar.containers import (
@@ -185,7 +186,8 @@ def validate_attestation(self, store: LstarStore, attestation_data: AttestationD
185186
3. The head must be at least as recent as source and target.
186187
4. Checkpoint slots must match the actual block slots.
187188
5. Source, target, and head must lie on one parent chain.
188-
6. The vote's slot must have started locally (a small disparity margin is allowed).
189+
6. The vote's slot cannot precede the slot of the head it claims to have seen.
190+
7. The vote's slot must have started locally (a small disparity margin is allowed).
189191
190192
Raises:
191193
SpecRejectionError: If the attestation fails any of the validation checks above.
@@ -259,6 +261,16 @@ def validate_attestation(self, store: LstarStore, attestation_data: AttestationD
259261
"Target checkpoint must be ancestor of head",
260262
)
261263

264+
# Head Consistency Check
265+
#
266+
# A vote cannot have observed its head before that head existed.
267+
# This lower bound also keeps the wire slot clear of the 2**64 overflow edge.
268+
if attestation_data.slot < head_checkpoint.slot:
269+
raise SpecRejectionError(
270+
RejectionReason.ATTESTATION_SLOT_BEFORE_HEAD,
271+
"Attestation slot precedes head",
272+
)
273+
262274
# Time Check
263275
#
264276
# Reject votes whose slot has not started, with a small clock-skew margin.
@@ -270,8 +282,12 @@ def validate_attestation(self, store: LstarStore, attestation_data: AttestationD
270282
# interval 45 -> admitted only by a whole-slot margin, a full slot early
271283
#
272284
# The early window lets an adversary pre-publish next-slot aggregates.
273-
attestation_start_interval = Interval.from_slot(attestation_data.slot)
274-
if attestation_start_interval > store.time + Interval(GOSSIP_DISPARITY_INTERVALS):
285+
#
286+
# Work in slot units, not intervals.
287+
# Multiplying a near-2**64 wire slot into intervals would overflow and crash first.
288+
admission_horizon_interval = int(store.time) + int(GOSSIP_DISPARITY_INTERVALS)
289+
max_admissible_slot = admission_horizon_interval // int(INTERVALS_PER_SLOT)
290+
if int(attestation_data.slot) > max_admissible_slot:
275291
raise SpecRejectionError(
276292
RejectionReason.ATTESTATION_TOO_FAR_IN_FUTURE, "Attestation too far in future"
277293
)
@@ -322,6 +338,7 @@ def on_gossip_attestation(
322338
# - their slots are ordered source <= target <= head,
323339
# - each checkpoint slot matches its block's actual slot from the store,
324340
# - source, target, and head lie on one parent chain,
341+
# - the vote's slot is not before the head block's slot,
325342
# - the vote's slot has already started locally with a small margin.
326343
self.validate_attestation(store, attestation_data)
327344

@@ -428,6 +445,7 @@ def on_gossip_aggregated_attestation(
428445
# - their slots are ordered source <= target <= head,
429446
# - each checkpoint slot matches its block's actual slot from the store,
430447
# - source, target, and head lie on one parent chain,
448+
# - the vote's slot is not before the head block's slot,
431449
# - the vote's slot has already started locally with a small margin.
432450
self.validate_attestation(store, attestation_data)
433451

tests/consensus/lstar/fork_choice/test_gossip_attestation_validation.py

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,118 @@ def test_attestation_head_on_sibling_fork_rejected(
10011001
)
10021002

10031003

1004+
def test_attestation_slot_near_uint64_max_rejected(
1005+
fork_choice_test: ForkChoiceTestFiller,
1006+
) -> None:
1007+
"""
1008+
A vote with a slot near the unsigned 64-bit ceiling is rejected without overflow.
1009+
1010+
Given
1011+
-----
1012+
- 4 validators.
1013+
- the chain:
1014+
block_1(1) -> block_2(2)
1015+
- local time is at slot 2.
1016+
1017+
When
1018+
----
1019+
- V1 gossips a vote naming target block_2 at slot 2.
1020+
- the vote's own slot is the largest unsigned 64-bit value.
1021+
1022+
Then
1023+
----
1024+
- validation fails with attestation too far in future.
1025+
1026+
Regression
1027+
----------
1028+
- an earlier rule multiplied the wire slot into intervals before the bound check.
1029+
- a near-ceiling slot overflowed the interval constructor and crashed the node.
1030+
"""
1031+
fork_choice_test(
1032+
steps=[
1033+
BlockStep(
1034+
block=BlockSpec(slot=Slot(1), label="block_1"),
1035+
checks=StoreChecks(head_slot=Slot(1)),
1036+
),
1037+
BlockStep(
1038+
block=BlockSpec(slot=Slot(2), label="block_2"),
1039+
checks=StoreChecks(head_slot=Slot(2)),
1040+
),
1041+
AttestationStep(
1042+
attestation=GossipAttestationSpec(
1043+
validator_index=ValidatorIndex(1),
1044+
slot=Slot(2**64 - 1),
1045+
target_slot=Slot(2),
1046+
target_root_label="block_2",
1047+
head_slot=Slot(2),
1048+
head_root_label="block_2",
1049+
valid_signature=False,
1050+
),
1051+
valid=False,
1052+
expected_rejection=ExpectedRejection(
1053+
reason=RejectionReason.ATTESTATION_TOO_FAR_IN_FUTURE,
1054+
message_substring="Attestation too far in future",
1055+
),
1056+
),
1057+
],
1058+
)
1059+
1060+
1061+
def test_attestation_slot_before_head_rejected(
1062+
fork_choice_test: ForkChoiceTestFiller,
1063+
) -> None:
1064+
"""
1065+
A vote whose slot precedes the slot of its head block is rejected.
1066+
1067+
Given
1068+
-----
1069+
- 4 validators.
1070+
- the chain:
1071+
block_1(1) -> block_2(2) -> block_3(3)
1072+
1073+
When
1074+
----
1075+
- V1 gossips a vote with target block_2 at slot 2 and head block_3 at slot 3.
1076+
- the vote's own slot is 2, before the head block's slot 3.
1077+
1078+
Then
1079+
----
1080+
- validation fails because the vote slot precedes the head it claims to have seen.
1081+
"""
1082+
fork_choice_test(
1083+
steps=[
1084+
BlockStep(
1085+
block=BlockSpec(slot=Slot(1), label="block_1"),
1086+
checks=StoreChecks(head_slot=Slot(1)),
1087+
),
1088+
BlockStep(
1089+
block=BlockSpec(slot=Slot(2), label="block_2"),
1090+
checks=StoreChecks(head_slot=Slot(2)),
1091+
),
1092+
BlockStep(
1093+
block=BlockSpec(slot=Slot(3), label="block_3"),
1094+
checks=StoreChecks(head_slot=Slot(3)),
1095+
),
1096+
AttestationStep(
1097+
attestation=GossipAttestationSpec(
1098+
validator_index=ValidatorIndex(1),
1099+
slot=Slot(2),
1100+
target_slot=Slot(2),
1101+
target_root_label="block_2",
1102+
head_slot=Slot(3),
1103+
head_root_label="block_3",
1104+
valid_signature=False,
1105+
),
1106+
valid=False,
1107+
expected_rejection=ExpectedRejection(
1108+
reason=RejectionReason.ATTESTATION_SLOT_BEFORE_HEAD,
1109+
message_substring="Attestation slot precedes head",
1110+
),
1111+
),
1112+
],
1113+
)
1114+
1115+
10041116
def test_attestation_source_on_sibling_fork_rejected(
10051117
fork_choice_test: ForkChoiceTestFiller,
10061118
) -> None:

0 commit comments

Comments
 (0)