onionmessage: drop onion messages cycling back to the sending peer
What changed, and why it matters
This update fixes a way that a malicious or misconfigured peer could make a Lightning node repeatedly bounce onion messages back to itself, wasting bandwidth and CPU. The node now detects when the next hop of an onion message is the same peer that sent it and drops the message instead of forwarding it.
Apply the patch. It is a targeted, low-risk hardening change. Monitor logs for ErrSamePeerCycle warnings to identify peers attempting or causing cyclic onion message routes.
Security signals we found
traffic amplification / loop prevention
denial-of-service mitigation
new defensive validation in message forwarding path
explicit error type introduced for detected cycles
Evidence from the diff
The commit adds a same-peer cycle check in onionmessage/actor.go. After resolving the routing action (whether by direct next-node public key or by short channel ID), it compares the resolved next hop’s node ID to the actor’s peerPubKey. If they match, it returns the new ErrSamePeerCycle error, logs a warning, and prevents forwarding or dispatching. Tests cover both direct next-node-ID and SCID-resolved paths.
Changed components
onionmessage/actor.goonionmessage/actor_test.goonionmessage/errors.goInspect captured patch +130 / −0
diff --git a/onionmessage/actor.go b/onionmessage/actor.go
index 355bee0..6a80bb7 100644
--- a/onionmessage/actor.go
+++ b/onionmessage/actor.go
@@ -188,6 +188,32 @@ func (a *OnionPeerActor) Receive(ctx context.Context,
return fn.Err[*Response](err)
}
+ // Block same-peer cycles: do not forward a message back to
+ // the peer that sent it.
+ routingAction.WhenLeft(func(fwdAction forwardAction) {
+ var nextNodeIDBytes [33]byte
+ copy(
+ nextNodeIDBytes[:],
+ fwdAction.nextNodeID.SerializeCompressed(),
+ )
+
+ if nextNodeIDBytes == a.peerPubKey {
+ log.WarnS(logCtx,
+ "Dropping cyclic onion message",
+ ErrSamePeerCycle,
+ lnutils.LogPubKey(
+ "next_node_id",
+ fwdAction.nextNodeID,
+ ),
+ )
+
+ err = ErrSamePeerCycle
+ }
+ })
+ if err != nil {
+ return fn.Err[*Response](err)
+ }
+
// Handle the routing action.
payload := fn.ElimEither(routingAction,
func(fwdAction forwardAction) *lnwire.OnionMessagePayload {
diff --git a/onionmessage/actor_test.go b/onionmessage/actor_test.go
index 7f5ea6c..3fb2e79 100644
--- a/onionmessage/actor_test.go
+++ b/onionmessage/actor_test.go
@@ -474,6 +474,105 @@ func TestOnionPeerActorRouting(t *testing.T) {
}
}
+// TestOnionPeerActorSamePeerCycle verifies that the actor rejects onion
+// messages whose next hop is the same peer that sent them. Both the direct
+// next-node-ID and the SCID-resolved paths are covered.
+func TestOnionPeerActorSamePeerCycle(t *testing.T) {
+ t.Parallel()
+
+ type nextNodeFn func(h *actorHarness,
+ pub *btcec.PublicKey) fn.Either[*btcec.PublicKey,
+ lnwire.ShortChannelID]
+
+ tests := []struct {
+ name string
+ nextNode nextNodeFn
+ }{
+ {
+ name: "via next node ID",
+ nextNode: func(_ *actorHarness,
+ pub *btcec.PublicKey) fn.Either[
+ *btcec.PublicKey, lnwire.ShortChannelID] {
+
+ return fn.NewLeft[*btcec.PublicKey,
+ lnwire.ShortChannelID](pub)
+ },
+ },
+ {
+ name: "via SCID",
+ nextNode: func(h *actorHarness,
+ pub *btcec.PublicKey) fn.Either[
+ *btcec.PublicKey, lnwire.ShortChannelID] {
+
+ scid := lnwire.NewShortChanIDFromInt(999)
+ h.resolver.addPeer(scid, pub)
+
+ return fn.NewRight[*btcec.PublicKey](scid)
+ },
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ h := newActorHarness(t)
+
+ // Generate a key for the next hop, then set the
+ // actor's peerPubKey to the same key to simulate
+ // the message arriving from that peer.
+ nextNodeKey, err := btcec.NewPrivateKey()
+ require.NoError(t, err)
+ nextNodePub := nextNodeKey.PubKey()
+
+ h.actor.peerPubKey = pubKeyToArray(nextNodePub)
+
+ nextNode := tc.nextNode(h, nextNodePub)
+ rdA := record.NewNonFinalBlindedRouteDataOnionMessage(
+ nextNode, nil, nil,
+ )
+ rdB := &record.BlindedRouteData{}
+
+ plainA := EncodeBlindedRouteData(t, rdA)
+ plainB := EncodeBlindedRouteData(t, rdB)
+ hops := []*sphinx.HopInfo{
+ {
+ NodePub: h.nodeKey.PubKey(),
+ PlainText: plainA,
+ },
+ {NodePub: nextNodePub, PlainText: plainB},
+ }
+
+ blindedPath := BuildBlindedPath(t, hops)
+ onionMsg, _ := BuildOnionMessage(t, blindedPath, nil)
+
+ req := &Request{msg: *onionMsg}
+ result := h.actor.Receive(t.Context(), req)
+
+ // The actor must return an error.
+ require.True(t, result.IsErr())
+ result.WhenErr(func(err error) {
+ require.ErrorIs(t, err, ErrSamePeerCycle)
+ })
+
+ // No message should have been forwarded.
+ select {
+ case <-h.sender.sent:
+ require.FailNow(t, "message should not have "+
+ "been forwarded back to the sending "+
+ "peer")
+ default:
+ }
+
+ // No update should have been dispatched.
+ select {
+ case <-h.dispatcher.updates:
+ require.FailNow(t, "update should not be "+
+ "dispatched for a cyclic message")
+ default:
+ }
+ })
+ }
+}
+
// TestOnionPeerActorReceiveContextCanceled tests that OnionPeerActor.Receive
// returns an error when the context is canceled.
func TestOnionPeerActorReceiveContextCanceled(t *testing.T) {
diff --git a/onionmessage/errors.go b/onionmessage/errors.go
index bccc3f0..ac79808 100644
--- a/onionmessage/errors.go
+++ b/onionmessage/errors.go
@@ -14,4 +14,9 @@ var (
// ErrSCIDEmpty is returned when the short channel ID is missing from
// the route data.
ErrSCIDEmpty = errors.New("short channel ID empty")
+
+ // ErrSamePeerCycle is returned when a forwarding onion message
+ // would be sent back to the same peer it was received from.
+ ErrSamePeerCycle = errors.New("onion message cycle: next " +
+ "hop is the sending peer")
)
Why this scored 65/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.