Skip to content

fix: guard nil syncer dereference in statesync Reactor.Receive (backport #2883)#2931

Merged
rootulp merged 2 commits intov0.39.x-celestiafrom
mergify/bp/v0.39.x-celestia/pr-2883
Apr 17, 2026
Merged

fix: guard nil syncer dereference in statesync Reactor.Receive (backport #2883)#2931
rootulp merged 2 commits intov0.39.x-celestiafrom
mergify/bp/v0.39.x-celestia/pr-2883

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Apr 16, 2026

Closes https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-239

Summary

  • A connected P2P peer could crash any non-state-syncing node (the default operating mode) by sending a single SnapshotsResponse with Chunks > MaxSnapshotChunks. The validateMsg error path in Reactor.Receive called r.syncer.RejectPeer() before checking whether r.syncer was nil, causing an unrecovered panic that terminates the node process.
  • Added a nil guard with mutex protection around the RejectPeer call so oversized snapshot messages are safely rejected when no state sync is active.
  • Added a regression test that confirms the panic without the fix and passes with it.

Test plan

  • New test TestReactor_Receive_OversizedSnapshotResponse_NilSyncer verifies no panic when r.syncer is nil and an oversized SnapshotsResponse is received
  • Full ./statesync/... test suite passes
  • go vet ./statesync/... clean

🤖 Generated with Claude Code


Open with Devin
This is an automatic backport of pull request #2883 done by [Mergify](https://mergify.com).
Open with Devin

Closes
https://dashboard.hackenproof.com/manager/companies/celestia/celestia/reports/CELESTIA-239

## Summary

- A connected P2P peer could crash any non-state-syncing node (the
default operating mode) by sending a single `SnapshotsResponse` with
`Chunks > MaxSnapshotChunks`. The `validateMsg` error path in
`Reactor.Receive` called `r.syncer.RejectPeer()` before checking whether
`r.syncer` was nil, causing an unrecovered panic that terminates the
node process.
- Added a nil guard with mutex protection around the `RejectPeer` call
so oversized snapshot messages are safely rejected when no state sync is
active.
- Added a regression test that confirms the panic without the fix and
passes with it.

## Test plan

- [x] New test `TestReactor_Receive_OversizedSnapshotResponse_NilSyncer`
verifies no panic when `r.syncer` is nil and an oversized
`SnapshotsResponse` is received
- [x] Full `./statesync/...` test suite passes
- [x] `go vet ./statesync/...` clean

🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- devin-review-badge-begin -->

---

<a
href="https://app.devin.ai/review/celestiaorg/celestia-core/pull/2883"
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>
<!-- devin-review-badge-end -->

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
(cherry picked from commit 2ac43b1)
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines +1 to +3
- `[statesync]` Guard against nil syncer dereference in Reactor.Receive
when processing oversized SnapshotsResponse messages while no state sync
is in progress.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changelog filename missing required issue/PR number prefix

Both CONTRIBUTING.md and CLAUDE.md mandate that changelog entries follow the naming pattern {issue-or-pr-number}-{description}.md. The file statesync-nil-syncer-panic.md is missing the PR number prefix. All released changelog entries in the repo consistently use a number prefix (e.g., 2913-mempool-user-tx-latency-metric.md, 3092-consensus-timeout-ticker-data-race.md). This PR is #2931, so the file should be named 2931-statesync-nil-syncer-panic.md.

Prompt for agents
The changelog file at .changelog/unreleased/bug-fixes/statesync-nil-syncer-panic.md needs to be renamed to include the PR number as a prefix, per the conventions in CONTRIBUTING.md and CLAUDE.md. The file should be renamed to .changelog/unreleased/bug-fixes/2931-statesync-nil-syncer-panic.md (or use the relevant issue number if one exists). This is a file rename, not a content change.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@rootulp rootulp self-assigned this Apr 17, 2026
@rootulp rootulp enabled auto-merge (squash) April 17, 2026 15:58
@rootulp rootulp disabled auto-merge April 17, 2026 18:00
@rootulp rootulp merged commit a9959fe into v0.39.x-celestia Apr 17, 2026
22 checks passed
@rootulp rootulp deleted the mergify/bp/v0.39.x-celestia/pr-2883 branch April 17, 2026 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants