Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rootulp merged 2 commits into from
Apr 17, 2026
+64 −1
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b8208aa
fix: guard nil syncer dereference in statesync Reactor.Receive (#2883)
rootulp Mar 31, 2026
96cc0b6
Merge branch 'v0.39.x-celestia' into mergify/bp/v0.39.x-celestia/pr-2883
rootulp Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/unreleased/bug-fixes/statesync-nil-syncer-panic.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- `[statesync]` Guard against nil syncer dereference in Reactor.Receive
when processing oversized SnapshotsResponse messages while no state sync
is in progress.
Comment on lines +1 to +3
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Apr 16, 2026

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.

6 changes: 5 additions & 1 deletion statesync/reactor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,11 @@ func (r *Reactor) Receive(e p2p.Envelope) {
err := validateMsg(e.Message, r.cfg.MaxSnapshotChunks)
if err != nil {
if errors.Is(err, ErrExceedsMaxSnapshotChunks) {
r.syncer.RejectPeer(e.Src)
r.mtx.RLock()
if r.syncer != nil {
r.syncer.RejectPeer(e.Src)
}
r.mtx.RUnlock()
}
r.Logger.Error("Invalid message", "peer", e.Src, "msg", e.Message, "err", err)
r.Switch.StopPeerForError(e.Src, err, r.String())
Expand Down
56 changes: 56 additions & 0 deletions statesync/reactor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package statesync

import (
"net"
"testing"
"time"

Expand Down Expand Up @@ -96,6 +97,61 @@ func TestReactor_Receive_ChunkRequest(t *testing.T) {
}
}

// TestReactor_Receive_OversizedSnapshotResponse_NilSyncer verifies that
// receiving a SnapshotsResponse with Chunks > MaxSnapshotChunks does not
// nil-dereference r.syncer when no state sync is in progress.
func TestReactor_Receive_OversizedSnapshotResponse_NilSyncer(t *testing.T) {
cfg := config.DefaultStateSyncConfig()
conn := &proxymocks.AppConnSnapshot{}

peer := &p2pmocks.Peer{}
peer.On("ID").Return(p2p.ID("attacker"))
peer.On("IsRunning").Return(true)
peer.On("Stop").Return(nil)
peer.On("FlushStop").Return()
peer.On("String").Return("peer-attacker")
peer.On("RemoteAddr").Return(&net.TCPAddr{IP: net.IP{127, 0, 0, 1}, Port: 26656})
peer.On("CloseConn").Return(nil)
peer.On("NodeInfo").Return(p2p.DefaultNodeInfo{})
peer.On("SetRemovalFailed").Return()
peer.On("GetRemovalFailed").Return(false)
peer.On("IsPersistent").Return(false)
peer.On("IsOutbound").Return(false)
peer.On("SocketAddr").Return(nil)
peer.On("HasIPChanged").Return(false)

r := NewReactor(*cfg, conn, nil, NopMetrics())

sw := p2p.MakeSwitch(
config.DefaultP2PConfig(),
0,
func(i int, sw *p2p.Switch) *p2p.Switch { return sw },
)
r.SetSwitch(sw)

err := r.Start()
require.NoError(t, err)
t.Cleanup(func() {
if err := r.Stop(); err != nil {
t.Error(err)
}
})

// This should NOT panic even though r.syncer is nil.
require.NotPanics(t, func() {
r.Receive(p2p.Envelope{
ChannelID: SnapshotChannel,
Src: peer,
Message: &ssproto.SnapshotsResponse{
Height: 1,
Format: 1,
Chunks: cfg.MaxSnapshotChunks + 1,
Hash: []byte{1},
},
})
})
}

func TestReactor_Receive_SnapshotsRequest(t *testing.T) {
testcases := map[string]struct {
snapshots []*abci.Snapshot
Expand Down
Loading