fix!: prevent OOM via unbounded Groth16 verifying key deserialization by rootulp · Pull Request #6754 · celestiaorg/celestia-app

rootulp · 2026-03-08T06:29:49Z

Fixes https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-214

Summary

Add ValidateGroth16Vkey() that performs two checks on Groth16Vkey in ValidateBasic() and genesis Validate() before passing it to gnark's deserializer:
1. Exact size check: Groth16VkeySize (396 bytes) — rejects payloads that are too short or too long
2. G1.K length prefix check: Groth16VkeyG1KLength (3) — reads the big-endian uint32 at bytes 288–291 and verifies it equals the expected value (nPublic + 1 for SP1's 2 public inputs)
Without both checks, a crafted payload can cause gnark-crypto to make([]G1Affine, 0xFFFFFFFF) (~256 GiB allocation), crashing any node during CheckTx with no authentication required
The size check alone is insufficient: an attacker can craft a 396-byte payload (passes size check) with an inflated G1.K length prefix that still triggers OOM
Add POC test demonstrating the underlying gnark vulnerability (64 MiB allocation from 292-byte input, skipped in -short mode)
Add regression tests in both msgs_test.go and genesis_test.go verifying malicious payloads are rejected before deserialization, including the 396-byte bypass variant

Test plan

go test -v ./x/zkism/types/ — all tests pass including new OOM payload rejection tests
go test -v ./x/zkism/internal/groth16/ — POC test confirms 65 MiB allocation from 292-byte input (233,472× amplification)
go test -v -short ./x/zkism/internal/groth16/ — POC test correctly skipped in short mode
make build-standalone — compiles cleanly

🤖 Generated with Claude Code

Add an exact size check (396 bytes) on Groth16Vkey in ValidateBasic() and genesis Validate() before passing it to gnark's deserializer. Without this check, an attacker-controlled uint32 length prefix in the serialized VK causes gnark-crypto to allocate up to ~256 GiB from a ~292-byte input, crashing any node during CheckTx with no authentication required. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

rootulp · 2026-03-08T06:31:05Z

x/zkism/internal/groth16/groth16_test.go

+// TestNewVerifyingKey_OOMVulnerability demonstrates that NewVerifyingKey is
+// vulnerable to an OOM attack. A crafted payload with a large G1.K array
+// length prefix causes gnark to allocate (sliceLen * 64) bytes before reading
+// any element data.
+//
+// The BN254 verifying key binary format encodes 6 curve points (288 bytes
+// total) followed by a uint32 length prefix for the G1.K slice. An attacker
+// can set this to 0xFFFFFFFF, triggering a ~256 GiB allocation from a
+// ~292-byte input.
+//
+// This test uses a moderate G1.K length (1M elements = ~64 MB) to safely
+// demonstrate the code path without crashing the test runner. The allocation
+// happens, then gnark fails with EOF because no element data follows. In
+// production with 0xFFFFFFFF, the allocation would be ~256 GiB and crash the
+// node.
+func TestNewVerifyingKey_OOMVulnerability(t *testing.T) {


I'm open to removing this before merging b/c it provides value for reviewers pre-merge but won't provide much value post-merge.

Claude used this to verify the bug before implementing a fix.

Fine with keeping it if you want. Could optionally always skip the test to avoid every make test-short allocating an additional 64MiB

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

x/zkism/types/genesis.go

The existing size check (Groth16VkeySize == 396) can be bypassed by crafting a payload that is exactly 396 bytes but has the internal G1.K length prefix set to 0xFFFFFFFF. This causes gnark to allocate ~256 GiB before returning an error. These tests reproduce the issue raised in PR review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The outer size check alone is insufficient: an attacker can craft a payload that is exactly Groth16VkeySize (396 bytes) but has the internal G1.K length prefix set to 0xFFFFFFFF, causing gnark to allocate ~256 GiB before returning an error. Add ValidateGroth16Vkey() that checks both the total size and the big-endian uint32 G1.K length at bytes 288-291, ensuring it equals the expected value of 3 (nPublic + 1 for SP1's 2 public inputs). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

damiannolan · 2026-03-08T22:57:09Z

I will take a deeper look at this tomorrow. It makes sense to me, but I don't know if allocation of 256GB is possible. It looks like some reasonable size limits were added in Consensys/gnark-crypto#759 for many of the primitives including that which is used here, right? bn254 prime fields. cc. @jonas089

// to avoid allocating too large slice when the header is tampered, we limit
// the maximum allocation. We set the target to 4GB. This incurs a performance
// hit when reading very large slices, but protects against OOM.
targetSize := uint64(1 << 32) // 4GB
if bits.UintSize == 32 {
    // reduce target size to 1GB on 32 bits architectures
    targetSize = uint64(1 << 30) // 1GB
}
maxAllocateSliceLength := targetSize / uint64(Bytes)

edit: I see the actual point of slice allocation now in ecc/bn254/marshal.go.

damiannolan

Looks like the PR has already been auto-merged. LGTM regardless..

damiannolan · 2026-03-09T09:59:36Z

x/zkism/internal/groth16/groth16_test.go

+// TestNewVerifyingKey_OOMVulnerability demonstrates that NewVerifyingKey is
+// vulnerable to an OOM attack. A crafted payload with a large G1.K array
+// length prefix causes gnark to allocate (sliceLen * 64) bytes before reading
+// any element data.
+//
+// The BN254 verifying key binary format encodes 6 curve points (288 bytes
+// total) followed by a uint32 length prefix for the G1.K slice. An attacker
+// can set this to 0xFFFFFFFF, triggering a ~256 GiB allocation from a
+// ~292-byte input.
+//
+// This test uses a moderate G1.K length (1M elements = ~64 MB) to safely
+// demonstrate the code path without crashing the test runner. The allocation
+// happens, then gnark fails with EOF because no element data follows. In
+// production with 0xFFFFFFFF, the allocation would be ~256 GiB and crash the
+// node.
+func TestNewVerifyingKey_OOMVulnerability(t *testing.T) {


Fine with keeping it if you want. Could optionally always skip the test to avoid every make test-short allocating an additional 64MiB

… (backport #6754) (#6781) Fixes https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-214 ## Summary - Add `ValidateGroth16Vkey()` that performs two checks on `Groth16Vkey` in `ValidateBasic()` and genesis `Validate()` **before** passing it to gnark's deserializer: 1. **Exact size check**: `Groth16VkeySize` (396 bytes) — rejects payloads that are too short or too long 2. **G1.K length prefix check**: `Groth16VkeyG1KLength` (3) — reads the big-endian `uint32` at bytes 288–291 and verifies it equals the expected value (`nPublic + 1` for SP1's 2 public inputs) - Without both checks, a crafted payload can cause gnark-crypto to `make([]G1Affine, 0xFFFFFFFF)` (~256 GiB allocation), crashing any node during `CheckTx` with no authentication required - The size check alone is insufficient: an attacker can craft a 396-byte payload (passes size check) with an inflated G1.K length prefix that still triggers OOM - Add POC test demonstrating the underlying gnark vulnerability (64 MiB allocation from 292-byte input, skipped in `-short` mode) - Add regression tests in both `msgs_test.go` and `genesis_test.go` verifying malicious payloads are rejected before deserialization, including the 396-byte bypass variant ## Test plan - [x] `go test -v ./x/zkism/types/` — all tests pass including new OOM payload rejection tests - [x] `go test -v ./x/zkism/internal/groth16/` — POC test confirms 65 MiB allocation from 292-byte input (233,472× amplification) - [x] `go test -v -short ./x/zkism/internal/groth16/` — POC test correctly skipped in short mode - [x] `make build-standalone` — compiles cleanly 🤖 Generated with [Claude Code](https://claude.com/claude-code)<hr>This is an automatic backport of pull request #6754 done by [Mergify](https://mergify.com).  --- <a href="https://app.devin.ai/review/celestiaorg/celestia-app/pull/6781" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a>  --------- Co-authored-by: Rootul Patel <rootulp@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

rootulp requested review from cmwaters, evan-forbes, ninabarbakadze and rach-id as code owners March 8, 2026 06:29

rootulp commented Mar 8, 2026

View reviewed changes

rootulp self-assigned this Mar 8, 2026

rootulp requested a review from damiannolan March 8, 2026 06:31

devin-ai-integration bot reviewed Mar 8, 2026

View reviewed changes

rach-id reviewed Mar 8, 2026

View reviewed changes

x/zkism/types/genesis.go Outdated Show resolved Hide resolved

rootulp marked this pull request as draft March 8, 2026 22:02

rootulp and others added 2 commits March 8, 2026 18:07

rootulp marked this pull request as ready for review March 8, 2026 22:17

rootulp requested a review from rach-id March 8, 2026 22:17

rootulp enabled auto-merge March 8, 2026 22:17

jonas089 approved these changes Mar 9, 2026

View reviewed changes

rootulp added this pull request to the merge queue Mar 9, 2026

Merged via the queue into main with commit 53c577c Mar 9, 2026
25 checks passed

rootulp deleted the fix/zkism-groth16-vkey-oom branch March 9, 2026 09:00

damiannolan reviewed Mar 9, 2026

View reviewed changes

rootulp mentioned this pull request Mar 9, 2026

Skip or remove TestNewVerifyingKey_OOMVulnerability #6773

Closed

rootulp added the backport:v7.x label Mar 11, 2026

mergify bot mentioned this pull request Mar 11, 2026

fix!: prevent OOM via unbounded Groth16 verifying key deserialization (backport #6754) #6781

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: prevent OOM via unbounded Groth16 verifying key deserialization#6754

fix!: prevent OOM via unbounded Groth16 verifying key deserialization#6754
rootulp merged 3 commits intomainfrom
fix/zkism-groth16-vkey-oom

rootulp commented Mar 8, 2026 •

edited

Loading

Uh oh!

rootulp Mar 8, 2026

Uh oh!

damiannolan Mar 9, 2026

Uh oh!

devin-ai-integration bot left a comment

Uh oh!

Uh oh!

damiannolan commented Mar 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

damiannolan left a comment

Uh oh!

damiannolan Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

rootulp commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

rootulp Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

damiannolan Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

Uh oh!

damiannolan commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

damiannolan left a comment

Choose a reason for hiding this comment

Uh oh!

damiannolan Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rootulp commented Mar 8, 2026 •

edited

Loading

damiannolan commented Mar 8, 2026 •

edited

Loading