erigontech / erigontech/erigon
cl/das: VerifyDataColumnSidecarKZGProofs fails open for Gloas sidecars
- 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
Assessment
This issue has not been assessed yet.