routing: Fix possible infinite loop on first hop misleading hint

Add ignore condition to additional edges that connect to self. These
edges are already known and avoiding these hints protect the payment
from malformed channel ids which could lead to infinite loop.

Fixes lightningnetwork#6169.

Co-authored-by: lsunsi <lsunsi@pm.me>

routing/integrated_routing_context_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/lightningnetwork/lnd/kvdb"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/route"
+	"github.com/lightningnetwork/lnd/zpay32"
 )
 
 const (
@@ -47,6 +48,7 @@ type integratedRoutingContext struct {
 
 	mcCfg          MissionControlConfig
 	pathFindingCfg PathFindingConfig
+	routeHints     [][]zpay32.HopHint
 }
 
 // newIntegratedRoutingContext instantiates a new integrated routing test
@@ -177,6 +179,7 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32,
 		Amount:         c.amt,
 		CltvLimit:      math.MaxUint32,
 		MaxParts:       maxParts,
+		RouteHints:     c.routeHints,
 	}
 
 	var paymentHash [32]byte

routing/integrated_routing_test.go

@@ -4,9 +4,11 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/zpay32"
 	"github.com/stretchr/testify/require"
 )
 
@@ -234,6 +236,34 @@ var mppTestCases = []mppSendTestCase{
 	},
 }
 
+// TestBadFirstHopHint tests that a payment with a first hop hint with an
+// invalid channel id still works since the node already knows its channel and
+// doesn't need hints.
+func TestBadFirstHopHint(t *testing.T) {
+	t.Parallel()
+
+	ctx := newIntegratedRoutingContext(t)
+
+	onePathGraph(ctx.graph)
+
+	sourcePubKey, _ := btcec.ParsePubKey(ctx.source.pubkey[:])
+
+	hopHint := zpay32.HopHint{
+		NodeID:                    sourcePubKey,
+		ChannelID:                 66,
+		FeeBaseMSat:               0,
+		FeeProportionalMillionths: 0,
+		CLTVExpiryDelta:           100,
+	}
+
+	ctx.routeHints = [][]zpay32.HopHint{{hopHint}}
+
+	ctx.amt = lnwire.NewMSatFromSatoshis(100)
+	_, err := ctx.testPayment(1)
+
+	require.NoError(t, err, "payment failed")
+}
+
 // TestMppSend tests that a payment can be completed using multiple shards.
 func TestMppSend(t *testing.T) {
 	for _, testCase := range mppTestCases {

routing/mock_graph_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -13,7 +14,12 @@ import (
 
 // createPubkey return a new test pubkey.
 func createPubkey(id byte) route.Vertex {
-	pubkey := route.Vertex{id}
+	_, secpPub := btcec.PrivKeyFromBytes([]byte{id})
+
+	var bytes [33]byte
+	copy(bytes[:], secpPub.SerializeCompressed()[:33])
+
+	pubkey := route.Vertex(bytes)
 	return pubkey
 }
 

routing/pathfind.go

@@ -538,10 +538,18 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 	additionalEdgesWithSrc := make(map[route.Vertex][]*edgePolicyWithSource)
 	for vertex, outgoingEdgePolicies := range g.additionalEdges {
+		// Edges connected to self are always included in the graph,
+		// therefore can be skipped. This prevents us from trying
+		// routes to malformed hop hints.
+		if vertex == self {
+			continue
+		}
+
 		// Build reverse lookup to find incoming edges. Needed because
 		// search is taken place from target to source.
 		for _, outgoingEdgePolicy := range outgoingEdgePolicies {
 			toVertex := outgoingEdgePolicy.ToNodePubKey()
+
 			incomingEdgePolicy := &edgePolicyWithSource{
 				sourceNode: vertex,
 				edge:       outgoingEdgePolicy,

routing/pathfind_test.go

@@ -773,6 +773,9 @@ func TestPathFinding(t *testing.T) {
 	}, {
 		name: "path finding with additional edges",
 		fn:   runPathFindingWithAdditionalEdges,
+	}, {
+		name: "path finding with redundant additional edges",
+		fn:   runPathFindingWithRedundantAdditionalEdges,
 	}, {
 		name: "new route path too long",
 		fn:   runNewRoutePathTooLong,
@@ -1290,6 +1293,58 @@ func runPathFindingWithAdditionalEdges(t *testing.T, useCache bool) {
 	assertExpectedPath(t, graph.aliasMap, path, "songoku", "doge")
 }
 
+// runPathFindingWithRedundantAdditionalEdges asserts that we are able to find
+// paths to nodes ignoring additional edges that are already known by self node.
+func runPathFindingWithRedundantAdditionalEdges(t *testing.T, useCache bool) {
+	t.Helper()
+
+	var realChannelID uint64 = 3145
+	var hintChannelID uint64 = 1618
+
+	testChannels := []*testChannel{
+		symmetricTestChannel("alice", "bob", 100000, &testChannelPolicy{
+			Expiry:  144,
+			FeeRate: 400,
+			MinHTLC: 1,
+			MaxHTLC: 100000000,
+		}, realChannelID),
+	}
+
+	ctx := newPathFindingTestContext(t, useCache, testChannels, "alice")
+
+	target := ctx.keyFromAlias("bob")
+	paymentAmt := lnwire.NewMSatFromSatoshis(100)
+
+	// Create the channel edge going from alice to bob and include it in
+	// our map of additional edges.
+	aliceToBob := &channeldb.CachedEdgePolicy{
+		ToNodePubKey: func() route.Vertex {
+			return target
+		},
+		ToNodeFeatures:            lnwire.EmptyFeatureVector(),
+		ChannelID:                 hintChannelID,
+		FeeBaseMSat:               1,
+		FeeProportionalMillionths: 1000,
+		TimeLockDelta:             9,
+	}
+
+	additionalEdges := map[route.Vertex][]*channeldb.CachedEdgePolicy{
+		ctx.source: {aliceToBob},
+	}
+
+	path, err := dbFindPath(
+		ctx.graph, additionalEdges, ctx.bandwidthHints,
+		&ctx.restrictParams, &ctx.pathFindingConfig, ctx.source, target,
+		paymentAmt, ctx.timePref, 0,
+	)
+
+	require.NoError(t, err, "unable to find path to bob")
+	require.Len(t, path, 1)
+
+	require.Equal(t, realChannelID, path[0].ChannelID,
+		"additional edge for known edge wasn't ignored")
+}
+
 // TestNewRoute tests whether the construction of hop payloads by newRoute
 // is executed correctly.
 func TestNewRoute(t *testing.T) {

routing/router_test.go

@@ -361,6 +361,79 @@ func TestSendPaymentRouteFailureFallback(t *testing.T) {
 	)
 }
 
+// TestSendPaymentRouteInfiniteLoopWithBadHopHint tests that when sending
+// a payment with a malformed hop hint in the first hop, the hint is ignored
+// and the payment succeeds without an infinite loop of retries.
+func TestSendPaymentRouteInfiniteLoopWithBadHopHint(t *testing.T) {
+	t.Parallel()
+
+	const startingBlockHeight = 101
+	ctx := createTestCtxFromFile(t, startingBlockHeight, basicGraphFilePath)
+
+	source := ctx.aliases["roasbeef"]
+	sourceNodeID, err := btcec.ParsePubKey(source[:])
+	require.NoError(t, err)
+
+	actualChannelID := ctx.getChannelIDFromAlias(t, "roasbeef", "songoku")
+	badChannelID := uint64(66666)
+
+	// Craft a LightningPayment struct that'll send a payment from roasbeef
+	// to songoku for 1000 satoshis.
+	var payHash lntypes.Hash
+	paymentAmt := lnwire.NewMSatFromSatoshis(1000)
+	payment := LightningPayment{
+		Target:      ctx.aliases["songoku"],
+		Amount:      paymentAmt,
+		FeeLimit:    noFeeLimit,
+		paymentHash: &payHash,
+		RouteHints: [][]zpay32.HopHint{{
+			zpay32.HopHint{
+				NodeID:          sourceNodeID,
+				ChannelID:       badChannelID,
+				FeeBaseMSat:     uint32(50),
+				CLTVExpiryDelta: uint16(200),
+			},
+		}},
+	}
+
+	var preImage [32]byte
+	copy(preImage[:], bytes.Repeat([]byte{9}, 32))
+
+	// Mock a payment result that always fails with FailUnknownNextPeer when
+	// the bad channel is the first hop.
+	badShortChanID := lnwire.NewShortChanIDFromInt(badChannelID)
+	newFwdError := htlcswitch.NewForwardingError(
+		&lnwire.FailUnknownNextPeer{}, 0,
+	)
+
+	payer, ok := ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcherOld)
+	require.Equal(t, ok, true, "failed Payer cast")
+
+	payer.setPaymentResult(
+		func(firstHop lnwire.ShortChannelID) ([32]byte, error) {
+			// Returns a FailUnknownNextPeer if it's trying
+			// to pay an invalid channel.
+			if firstHop == badShortChanID {
+				return [32]byte{}, newFwdError
+			}
+
+			return preImage, nil
+		})
+
+	// Send off the payment request to the router, should succeed
+	// ignoring the bad channel id hint.
+	paymentPreImage, route, paymentErr := ctx.router.SendPayment(&payment)
+	require.NoError(t, paymentErr, "payment returned an error")
+
+	// The preimage should match up with the one created above.
+	require.Equal(t, preImage[:], paymentPreImage[:], "incorrect preimage")
+
+	// The route should have songoku as the first hop.
+	require.Equal(t, actualChannelID, route.Hops[0].ChannelID,
+		"route should go through the correct channel id",
+	)
+}
+
 // TestChannelUpdateValidation tests that a failed payment with an associated
 // channel update will only be applied to the graph when the update contains a
 // valid signature.