erigontech / erigontech/erigon

cl/das: VerifyDataColumnSidecarKZGProofs fails open for Gloas sidecars

Open
#22,805 1 comment 0 reactions 1 assignee Claimed by @domiwei View on GitHub
Caplin Glamsterdam
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## Problem

`VerifyDataColumnSidecarKZGProofs` (`cl/das/p2p_utils.go:92-99`) returns `true` without verifying anything when the sidecar was decoded with the Gloas schema:

```go
func VerifyDataColumnSidecarKZGProofs(sidecar *cltypes.DataColumnSidecar) bool {
if sidecar.Version() >= clparams.GloasVersion {
// GLOAS sidecars don't have KzgCommitments in the sidecar itself
// Caller should use VerifyDataColumnSidecarKZGProofsWithCommitments with external commitments
return true // Skip for now, caller must use the WithCommitments variant
}
```

The precondition is real — Gloas moved the commitments to the block's bid, so this function genuinely cannot verify a Gloas sidecar. But expressing that as `return true` from a function named `Verify...` means any caller that reaches it with a Gloas sidecar gets a silent pass rather than an error.

## Why it is not currently exploitable

`runDownload` dispatches on `sidecar.Version()` and routes Gloas sidecars to `VerifyDataColumnSidecarKZGProofsWithCommitments`, and since #22797 a sidecar whose slot-implied fork disagrees with its decoded schema is rejected before reaching verification at all. So there is no known path today that reaches the fail-open branch with data it should have checked. The concern is the next caller.

This is worth fixing precisely because the surrounding code is easy to get wrong: `VerifyDataColumnSidecarInclusionProof` (`p2p_utils.go:172-178`) also returns `true` for Gloas, but there it is correct — Gloas removed the inclusion proof, so there is nothing to verify. Two adjacent functions returning `true` for Gloas, one legitimately and one as a placeholder, is a trap.

## Suggestion

Make the precondition impossible to ignore rather than documented in a comment: return `(bool, error)` and error for Gloas, or rename so the constraint is in the signature (e.g. `VerifyPreGloasDataColumnSidecarKZGProofs`), so a caller that forgets the `WithCommitments` variant fails to compile or fails loudly.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.