Skip to content

cl/utils/eth_clock: unchecked uint64 arithmetic in slot-time conversions lets a forged slot alias to the current slot #24360

Description

@lystopad

Follow-up to #24259 (background sync-committee gossip publish), flagged by domiwei during review, explicitly out of scope for that PR.

Problem

EthereumClock.GetSlotTime and IsSlotCurrentSlotWithMaximumClockDisparity (cl/utils/eth_clock/ethereum_clock.go) use unchecked uint64 arithmetic (genesisTime + SecondsPerSlot*slot, and the reverse conversion in GetSlotByTime). On mainnet, a large but bounded slot value - domiwei's example: 4611686018442700405 - wraps under this arithmetic to a timestamp that maps back to the current slot, so IsSlotCurrentSlotWithMaximumClockDisparity returns true for it.

cl/phase1/network/services/sync_committee_messages_service.go's ProcessMessage uses exactly this check as its [IGNORE] gate before signature verification. The sync-committee message signature does not cover the slot field, so a forged slot aliasing to "now" can pass this check, enter the pool, and be forwarded over gossip - independent of #24259's own admission-side guard (maxFutureSlotLookahead in cl/beacon/handler/pool.go), which only bounds what reaches the publish side, not what ProcessMessage/the gossip validator itself accepts.

Scope note

Not introduced by #24259 - the underlying arithmetic pre-dates it. #24259's own syncCommitteeMessageExpiry guard already handles the equivalent overflow on the publish-admission side (see its maxFutureSlotLookahead bound and TestSyncCommitteeMessageExpiryDoesNotAliasToFutureForHugeSlot), but that guard has no effect here since this is a different call path (gossip/pool ingestion, not REST-triggered publish).

Candidate direction (not yet implemented/reviewed)

Bound-check slot inputs before any slot-to-time arithmetic in eth_clock (analogous to the guard added in #24259), or reject slots beyond a sane lookahead earlier in the validation pipeline, before IsSlotCurrentSlotWithMaximumClockDisparity is ever reached. Since GetSlotTime/GetSlotByTime are used broadly across the codebase, a fix likely belongs in eth_clock itself rather than at each call site.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

CaplinCaplin: Consensus Layer, Beacon API

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions