cl: reject sidecars and blocks with inconsistent fork schema - #22797
Merged
yperbasis merged 6 commits intoJul 29, 2026
Conversation
A data-column sidecar is decoded with the SSZ schema selected by the response fork-context digest, which the sending peer controls. The PeerDAS downloader then re-derived the schema a second time from the sidecar's embedded slot: a peer could return a Gloas-schema sidecar (no SignedBlockHeader) whose slot maps to a pre-Gloas fork, taking the downloader down the pre-Gloas branch where it unconditionally dereferenced the absent SignedBlockHeader and crashed the process from an unrecovered goroutine. Resolve the slot and block root from the fields the decoded schema populates, and reject any sidecar whose slot-implied fork disagrees with its decoded schema across the Gloas boundary (banning the peer), before touching any schema-specific field.
yperbasis
added
Caplin
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a remote crash vector in Caplin’s PeerDAS column-sidecar downloader by ensuring the slot and block root are derived from fields that are actually present in the decoded SSZ schema, and rejecting sidecars whose slot-implied fork is inconsistent with the fork schema selected by the peer-controlled response digest.
Changes:
- Replaces slot/root derivation logic in
runDownloadwith a version-aware resolver that validates fork-schema consistency across the Gloas boundary. - Adds
resolveColumnSidecarSlotAndRoothelper to safely extract slot + block root and reject malformed/mismatched sidecars early (before schema-specific dereferences). - Adds unit tests covering acceptance and rejection scenarios around the Gloas fork boundary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cl/das/peer_das.go | Uses schema-aware slot/root resolution and rejects inconsistent/malformed sidecars to prevent nil-deref panics. |
| cl/das/peer_das_test.go | Adds targeted tests for the new resolver across Fulu/Gloas consistency and rejection cases. |
Comments suppressed due to low confidence (1)
cl/das/peer_das.go:927
- The doc comment claims ok=false is only about slot-fork vs decoded-schema consistency, but the function also rejects malformed pre-Gloas sidecars (nil SignedBlockHeader) and HashSSZ errors. Please adjust the comment so it matches the actual behavior.
// resolveColumnSidecarSlotAndRoot reads a received column sidecar's slot and
// block root from the fields populated by the schema it was decoded with. A peer
// picks that schema via the response fork-digest, independently of the slot the
// sidecar claims, so it returns ok=false unless the fork implied by the slot
// agrees with the decoded schema across the Gloas boundary (which removed
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback: resolveColumnSidecarSlotAndRoot rejects a malformed sidecar (nil SignedBlockHeader/Header, HashSSZ error) in addition to a slot/schema fork mismatch, so the rejection log and the doc comment no longer describe the mismatch as the only cause.
OnBlock derived a block's fork twice from two independent attacker-influenced inputs: blockVersion from the block's slot, and the decoded schema from the response fork-context digest, which the sending peer controls. Gloas moved ExecutionPayload and BlobKzgCommitments out of BeaconBody, so a Gloas-decoded block claiming a pre-Gloas slot took the pre-Gloas branch and dereferenced the absent BlobKzgCommitments. The deref happens before the version cross-check in ProcessBlock and before signature validation, so an unsigned crafted block reaches it. This is the same schema-confusion primitive as the data-column sidecar issue fixed earlier in this branch, reached through the block download path instead. Resolved the same way: require the fork implied by the slot to agree with the decoded schema across the Gloas boundary before either branch reads a version-specific field. Honest peers always agree, since a req/resp server encodes each block with the fork digest of that block's own slot. Found while auditing the sidecar fix against ethereum-bounty/erigon#19.
yperbasis
changed the title
yperbasis
changed the title
TestResolveColumnSidecarSlotAndRoot exercises only the extracted resolver, so a dereference reintroduced inside runDownload's per-sidecar goroutine would go unnoticed. Drive the real BeaconRpcP2P decode path instead, with a sentinel that serves a Gloas-digest sidecar claiming a pre-Gloas slot, and assert the peer is banned and nothing reaches column storage. That also pins the premise the fix rests on: DataColumnSidecar.Version() reflects the peer's digest choice. Adapted from the reporter's harness in ethereum-bounty/erigon#19. Verified red-on-revert: the pre-fix resolution logic panics in the wg.Go goroutine, which has no recover. The test gives peerdas a real PeerDasState and an in-memory column store so the half-check ticker cannot nil-panic, then waits for the ban on a channel rather than on a timeout, keeping it free of timing races. InitGlobalStaticConfig panics on a second call, so both tests now share initTestBeaconConfig, which initializes only when unset. This keeps the package order-independent; the two tests differ only in fork epochs, which each reads from its own local config.
This was referenced Jul 28, 2026
The comment on the OnBlock check overstated the hazard: it claimed either branch would read fields left unset, but validateParentPayloadPath already nil-guards the bid, so only the pre-Gloas path dereferences. The check stays symmetric — an inconsistent block is rejected either way — but the rationale now says what is actually true. Extracting the predicate as forkSchemaMatchesSlot names the invariant and makes both directions testable without a fork-choice store. It takes the already computed slot-derived version, so there is no extra GetCurrentStateVersion call on the block path. That replaces TestOnBlockRejectsPreGloasSchemaAtGloasSlot, which reassigned store.beaconCfg after construction so the block's slot would imply Gloas. Threading a config into buildExAnteStore is not an alternative: the fixtures are Deneb-era and CachingBeaconState.Version() derives from fork epochs, so an all-forks-at-genesis config makes the anchor state report Gloas and the Deneb blocks fail ProcessBlock's version check. The table test needs no store at all, and its both-pre-Gloas case documents that disagreements below the boundary are intentionally allowed. The OnBlock test stays for the crash direction. Both remain red-on-revert: removing the check panics in solid.ListSSZ.Len, and narrowing the predicate to one direction fails the mirror case. initTestBeaconConfig moves to peer_das_test.go, the package's general test file.
The predicate deciding whether an object's decoded SSZ schema agrees with the fork implied by its slot was hand-written in two packages. Hoist it to BeaconChainConfig.ForkSchemaMatchesSlot, which both now call, so the invariant has one name and one rationale rather than two copies of the expression. It takes a slot rather than a pre-computed version, so the two call sites read alike. In OnBlock that costs one extra GetCurrentStateVersion per block — a seven-iteration loop of integer comparisons, next to an SSZ merkleization — and lets the check move above the blockEpoch/blockVersion computation it no longer needs. computeEpochAtSlot is slot/SlotsPerEpoch, the same epoch the helper derives, so the check is unchanged. resolveColumnSidecarSlotAndRoot becomes one self-contained branch per fork instead of branching on isGloas three times, and its doc comment now points at the helper rather than restating the rationale. Behaviour is identical: the helper expands to the previous inline condition, and the nil guard still precedes it. The predicate's table test moves to cl/clparams alongside it, re-expressed in slots. A second test pins the mainnet case: with Gloas at FAR_FUTURE_EPOCH no slot maps to Gloas, so a Gloas-schema object is inconsistent whatever slot it claims — the condition the mainnet attack turns on. The download test's peerdas gets a real gloasDataCache. Without it a sidecar wrongly accepted as Gloas nil-panicked in the Gloas verification path instead of failing the ban assertion, hiding what had regressed.
This was referenced Jul 28, 2026
yperbasis
marked this pull request as ready for review
July 28, 2026 14:17
AskAlexSharov
approved these changes
Jul 29, 2026
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Jul 29, 2026
…ech#22810) Removes three unreachable functions from the Caplin PeerDAS code. No behaviour change. Independent of erigontech#22797 — based on `main`, not stacked on it — though the second removal is motivated by it. ## `SendColumnSidecarsByRangeReqV1` (`cl/rpc`) No callers. The column downloader only ever issues the by-root request (`SendColumnSidecarsByRootIdentifierReq`). Removed along with its `BeaconRpc` declaration; no generated mocks implement that interface, so nothing needed regenerating. The by-range **server** handler is untouched, so `DataColumnSidecarsByRangeProtocolV1` and `ColumnSidecarsByRangeRequest` remain in use — this only drops the unused client side. `columnSidecarRawBytes` also stays, since the by-root path uses it. ## `ValidatePartialDataColumnHeader` and its two helpers (`cl/phase1/network/services`) No callers either, and it was the sole caller of `validateGloasPartialHeader` and `validateFuluPartialHeader`, so all three go. The Fulu variant ends with ```go _ = blockRoot // Used for logging in production ``` which suggests it was never wired up. Worth removing rather than leaving: `PartialDataColumnHeader` carries the same dual Gloas schema as `DataColumnSidecar`, where conflating the decoded schema with the fork implied by the slot was a remotely triggerable crash (fixed in erigontech#22797). An unused validator for that type is a surface with the same hazard built in. `PartialDataColumnHeader` itself and `verifyProposerSignature` are untouched — both have live users, the latter on the gossip validation path. ## Checks `go build ./cl/...`, `make lint` (clean on two runs), and the `cl/rpc`, `cl/phase1/network/services` and `cl/das` test packages all pass. Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The SSZ schema a req/resp response decodes with is chosen by the peer, via the fork-context digest it sends. Two consumers derived the fork a second time from the object's own slot and read whichever fields that second inference implied. Where the two disagree across the Gloas boundary, the branch taken reads a field the decoded schema never populated — a nil dereference, in paths with no
recover.NewEthereumClockregisters every configured fork digest, including forks scheduled atFAR_FUTURE_EPOCH, so a peer can select the Gloas decoder on mainnet today.Data column sidecars (
cl/das/peer_das.go). Gloas removedSignedBlockHeaderfromDataColumnSidecar, replacing it withSlotandBeaconBlockRoot. A peer answersDataColumnSidecarsByRootwith the Gloas digest soSignedBlockHeaderdecodes as nil, sets the embeddedSlotto a pre-Gloas value, and the pre-Gloas branch dereferences the absent header. This runs in a per-sidecarwg.Gogoroutine, ahead of all KZG, inclusion-proof and signature validation. Requires a post-Fulu, pre-Gloas network and a node with a column download in flight, i.e. one missing a custody column.Beacon blocks (
cl/phase1/forkchoice/on_block.go). Gloas movedExecutionPayloadandBlobKzgCommitmentsout ofBeaconBody.OnBlockgated its pre-Gloas branch on the slot-derived version while the reads inside that branch are gated on the decodedblock.Version(), so a Gloas-digest block at a pre-Gloas slot reaches several nil reads —solid.ListSSZhas no nil-receiver guard. It precedes the pipeline's only version cross-check (ProcessBlock) and all signature validation, so an unsigned crafted block suffices. Requires a pre-Gloas network with Gloas in the fork schedule, a Deneb-or-later block, andnewPayloadwith an execution engine attached. Nothing need be in flight, so wider than the sidecar path.Impact in both cases is a remote liveness kill: process exit, Beacon API down until restart. Caplin-only; no consensus split, state corruption or code execution.
Fix
Both sites require the fork implied by the slot to agree with the decoded schema across the Gloas boundary before reading any schema-specific field. The two inferences are independent peer-influenced inputs, so they are checked against each other rather than trusted separately.
The check is
BeaconChainConfig.ForkSchemaMatchesSlot(slot, decodedVersion), shared by both call sites so the invariant carries one name and one rationale rather than two copies of the expression.cl/das:resolveColumnSidecarSlotAndRoottakes slot and block root only from the fields the decoded schema populates —sidecar.Version()is the structural truth — and bans the peer on a mismatch or a malformed sidecar.cl/phase1/forkchoice:OnBlockreturnsErrForkSchemaSlotMismatch. Both directions are rejected, keeping schema dispatch below the check uniform.Honest traffic is unaffected: a req/resp server encodes each object with the fork digest of that object's own slot, so decode-fork and slot-fork always agree unless the peer is lying. Disagreement below the Gloas boundary is allowed, since no double inference exists there. Consensus spec fixtures are unaffected because
ReadBeaconStateactivates every fork up to the test's version at epoch 0, matching the slot-derived version to the decoded one.Tests
TestResolveColumnSidecarSlotAndRoot— the sidecar resolver's accept and reject cases.TestRunDownloadRejectsGloasSidecarWithPreGloasSlot— a malicious sentinel driven through the realBeaconRpcP2Pdecode path intorunDownload; asserts the peer is banned and nothing reaches column storage.TestOnBlockRejectsForkSchemaSlotMismatch— a Gloas-decoded block at a pre-Gloas slot through the realForkChoiceStore.OnBlock.TestForkSchemaMatchesSlot— the shared predicate in both mismatch directions and both agreement cases, including that a disagreement below the Gloas boundary is not a mismatch.TestForkSchemaMatchesSlotFarFutureGloas— the mainnet shape: with Gloas atFAR_FUTURE_EPOCHno slot maps to it, so a Gloas-schema object is inconsistent whatever slot it claims.Each fails if the check it covers is removed.
Credit
The data-column sidecar issue was reported through the Ethereum bug bounty program (private PoC,
ethereum-bounty/erigon#19).