What changed, and why it matters
This commit fixes a deadlock bug in LND's gossip message handler. When a malformed channel announcement was processed, two error messages could be sent on a channel that only had room for one. Because some callers never read from that channel, the second send would block forever. This prevented LND from shutting down cleanly. The fix simply increases the channel buffer from 1 to 2. The bug was found during fuzz testing and is not described by the vendor as a security vulnerability.
Treat as a reliability/DoS-hardening fix rather than a critical security vulnerability. Apply the patch and monitor for the planned actor-model refactor referenced in the TODO. Operators concerned about clean shutdown should upgrade; no immediate exploit requiring emergency response is evident from the commit alone.
Security signals we found
Denial-of-service via shutdown deadlock
Malformed P2P gossip message triggers stuck goroutine
No authentication or rate-limiting bypass evident
Fix is a minimal buffer-size workaround, not a structural redesign
Evidence from the diff
In discovery/gossiper.go, ProcessRemoteAnnouncement creates an errChan to return errors from goroutines handling remote announcements. Previously buffered to 1, the channel could receive two writes (e.g., on wrong-chain rejection and on SignalDependents error) while callers such as Brontide ignore the returned channel. The second write blocked the goroutine, leaving a waitgroup incomplete and causing Stop() to hang. The patch changes the buffer to 2 and adds a regression test using a ChannelAnnouncement1 with a wrong chain hash and identical NodeID1/NodeID2.
Changed components
discovery/gossiper.godiscovery/gossiper_test.goAuthenticatedGossiper.ProcessRemoteAnnouncementAuthenticatedGossiper.StopInspect captured patch +69 / −1
diff --git a/discovery/gossiper.go b/discovery/gossiper.go
index 9d30083..ee83d50 100644
--- a/discovery/gossiper.go
+++ b/discovery/gossiper.go
@@ -880,7 +880,14 @@ func (d *AuthenticatedGossiper) stop() {
func (d *AuthenticatedGossiper) ProcessRemoteAnnouncement(ctx context.Context,
msg lnwire.Message, peer lnpeer.Peer) chan error {
- errChan := make(chan error, 1)
+ // Buffer up to two messages on errChan since up to two messages may be
+ // written and not all callers of this function actually read from
+ // errChan. Without this buffer goroutines end up blocking on writes to
+ // errChan, which prevents the gossiper from shutting down cleanly.
+ //
+ // TODO(ziggie): Redesign this once the actor model pattern becomes
+ // available. See https://github.com/lightningnetwork/lnd/pull/9820.
+ errChan := make(chan error, 2)
// For messages in the known set of channel series queries, we'll
// dispatch the message directly to the GossipSyncer, and skip the main
diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go
index 3d114e3..018eb3b 100644
--- a/discovery/gossiper_test.go
+++ b/discovery/gossiper_test.go
@@ -5236,3 +5236,64 @@ func TestRecoverGossipPanicNilJobID(t *testing.T) {
t.Fatal("timeout waiting for error")
}
}
+
+// TestGossiperShutdownWrongChainAnnouncement tests that the gossiper can shut
+// down cleanly after processing a channel announcement with the wrong chain
+// hash. This is a regression test for a bug where the gossiper would deadlock
+// on shutdown because more errors were sent on the error channel than it would
+// buffer, and no one was reading those error messages.
+//
+// In this test we trigger the sending of two error messages:
+// 1. First send when rejecting the wrong-chain announcement
+// 2. Second send when SignalDependents returns an error
+//
+// Since the error channel had a buffer of 1, the second send would block
+// forever, preventing the goroutine from completing and causing Stop() to hang
+// on wg.Wait().
+func TestGossiperShutdownWrongChainAnnouncement(t *testing.T) {
+ t.Parallel()
+
+ // Create a test context with the gossiper configured for MainNet.
+ tCtx, err := createTestCtx(t, 0, false)
+ require.NoError(t, err)
+
+ // Create a channel announcement with:
+ // 1. Wrong chain hash (SimNet instead of MainNet)
+ // 2. NodeID1 == NodeID2
+ //
+ // The first condition triggers the first error message to be sent, and
+ // the second condition causes SignalDependents to attempt to remove the
+ // same dependent job twice, which then triggers the second error
+ // message to be sent.
+ wrongChainAnn := &lnwire.ChannelAnnouncement1{
+ ChainHash: *chaincfg.SimNetParams.GenesisHash,
+ ShortChannelID: lnwire.ShortChannelID{
+ BlockHeight: 1,
+ TxIndex: 0,
+ TxPosition: 0,
+ },
+ Features: testFeatures,
+ }
+ // Use the SAME public key for NodeID1 and NodeID2 to trigger the
+ // second error message.
+ copy(wrongChainAnn.NodeID1[:], remoteKeyPub1.SerializeCompressed())
+ copy(wrongChainAnn.NodeID2[:], remoteKeyPub1.SerializeCompressed())
+ copy(wrongChainAnn.BitcoinKey1[:], bitcoinKeyPub1.SerializeCompressed())
+ copy(wrongChainAnn.BitcoinKey2[:], bitcoinKeyPub2.SerializeCompressed())
+
+ nodePeer := &mockPeer{remoteKeyPub1, nil, nil, atomic.Bool{}}
+
+ // Process the announcement without reading from the error channel,
+ // exactly as Brontide does.
+ _ = tCtx.gossiper.ProcessRemoteAnnouncement(
+ t.Context(), wrongChainAnn, nodePeer,
+ )
+
+ // Give the gossiper time to process the announcement.
+ time.Sleep(100 * time.Millisecond)
+
+ // Now stop the gossiper. This should complete without hanging.
+ // If the bug is present, Stop() will hang forever because a goroutine
+ // is blocked trying to send to the error channel a second time.
+ require.NoError(t, tCtx.gossiper.Stop())
+}
Why this scored 40/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.