From 239acb406395cb6af9196ad66a3e243396241fbe Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Wed, 4 Jun 2025 08:25:58 +0200
Subject: [PATCH] graph/db: validate edge policy TLV data before persisting

In this commit, we start validating the extra opaque data of a channel
edge policy before persisting it. We just check that the data is valid
TLV.

NOTE: we recently [started
validating](https://github.com/lightningnetwork/lnd/commit/1410a0949d555e14de11faf81a17fe64917505e3)
this at the lnwire level. So really, no new update will reach the DB
layer without this already being checked. But we check it again here so
that the DB API behaves correctly as its own unit.
---
 graph/db/errors.go     |  4 ++++
 graph/db/graph_test.go | 18 ++++++++++++------
 graph/db/kv_store.go   |  6 ++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/graph/db/errors.go b/graph/db/errors.go
index afdbdb457..8201980aa 100644
--- a/graph/db/errors.go
+++ b/graph/db/errors.go
@@ -12,6 +12,10 @@ var (
 	ErrEdgePolicyOptionalFieldNotFound = fmt.Errorf("optional field not " +
 		"present")
 
+	// ErrParsingExtraTLVBytes is returned when we attempt to parse
+	// extra opaque bytes as a TLV stream, but the parsing fails.
+	ErrParsingExtraTLVBytes = fmt.Errorf("error parsing extra TLV bytes")
+
 	// ErrGraphNotFound is returned when at least one of the components of
 	// graph doesn't exist.
 	ErrGraphNotFound = fmt.Errorf("graph bucket not initialized")
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index c8a5c54b2..efe1b4642 100644
--- a/graph/db/graph_test.go
+++ b/graph/db/graph_test.go
@@ -4195,19 +4195,25 @@ func TestGraphCacheForEachNodeChannel(t *testing.T) {
 
 	directedChan := getSingleChannel()
 	require.NotNil(t, directedChan)
-	require.Equal(t, directedChan.InboundFee, lnwire.Fee{
+	expectedInbound := lnwire.Fee{
 		BaseFee: 10,
 		FeeRate: 20,
-	})
+	}
+	require.Equal(t, expectedInbound, directedChan.InboundFee)
 
-	// Set an invalid inbound fee and check that the edge is no longer
-	// returned.
+	// Set an invalid inbound fee and check that persistence fails.
 	edge1.ExtraOpaqueData = []byte{
 		253, 217, 3, 8, 0,
 	}
-	require.NoError(t, graph.UpdateEdgePolicy(edge1))
+	require.ErrorIs(
+		t, graph.UpdateEdgePolicy(edge1), ErrParsingExtraTLVBytes,
+	)
 
-	require.Nil(t, getSingleChannel())
+	// Since persistence of the last update failed, we should still bet
+	// the previous result when we query the channel again.
+	directedChan = getSingleChannel()
+	require.NotNil(t, directedChan)
+	require.Equal(t, expectedInbound, directedChan.InboundFee)
 }
 
 // TestGraphLoading asserts that the cache is properly reconstructed after a
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index b08c86022..f68732e81 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -4546,6 +4546,12 @@ func serializeChanEdgePolicy(w io.Writer, edge *models.ChannelEdgePolicy,
 		}
 	}
 
+	// Validate that the ExtraOpaqueData is in fact a valid TLV stream.
+	err = edge.ExtraOpaqueData.ValidateTLV()
+	if err != nil {
+		return fmt.Errorf("%w: %w", ErrParsingExtraTLVBytes, err)
+	}
+
 	if len(edge.ExtraOpaqueData) > MaxAllowedExtraOpaqueBytes {
 		return ErrTooManyExtraOpaqueBytes(len(edge.ExtraOpaqueData))
 	}