multi: add restriction on maximum feature bit in invoices

Base 32 encoded bolt 11 invoices only allow 10 bits to express the
length of the feature vector in a tagged field, so there is a much
lower limit on the values invoice custom features can hold.

Other places in the protocol are theoretically limited by the maximum
message size, but since we express a feature bit as u16 we don't need
to be concerned about this.

The decision is made to track maximum per-set in the feature manager,
which is conceptually aware of sets and then validate in lnwire/features
against some arbitrary maximum value provided to the caller to keep
the base features package unaware of sets.

feature/manager.go

@@ -221,7 +221,9 @@ func (m *Manager) UpdateFeatureSets(
 			return err
 		}
 
-		if err := m.Get(set).ValidateUpdate(newFeatures); err != nil {
+		if err := m.Get(set).ValidateUpdate(
+			newFeatures, set.Maximum(),
+		); err != nil {
 			return err
 		}
 

feature/set.go

@@ -1,5 +1,11 @@
 package feature
 
+import (
+	"math"
+
+	"github.com/lightningnetwork/lnd/lnwire"
+)
+
 // Set is an enum identifying various feature sets, which separates the single
 // feature namespace into distinct categories depending what context a feature
 // vector is being used.
@@ -56,3 +62,22 @@ func (s Set) String() string {
 		return "SetUnknown"
 	}
 }
+
+// Maximum returns the maximum allowable value for a feature bit in the context
+// of a set. The maximum feature value we can express differs by set context
+// because the amount of space available varies between protocol messages. In
+// practice this should never be a problem (reasonably one would never hit
+// these high ranges), but we enforce these maximums for the sake of sane
+// validation.
+func (s Set) Maximum() lnwire.FeatureBit {
+	switch s {
+	case SetInvoice, SetInvoiceAmp:
+		return lnwire.MaxBolt11Feature
+
+	// The space available in other sets is > math.MaxUint16, so we just
+	// return the maximum value our expression of a feature bit allows so
+	// that any value will pass.
+	default:
+		return math.MaxUint16
+	}
+}

lnwire/features.go

@@ -16,6 +16,10 @@ var (
 	// set of features are made.
 	ErrFeatureStandard = errors.New("feature is used in standard " +
 		"protocol set")
+
+	// ErrFeatureBitMaximum is returned when a feature bit exceeds the
+	// maximum allowable value.
+	ErrFeatureBitMaximum = errors.New("feature bit exceeds allowed maximum")
 )
 
 // FeatureBit represents a feature that can be enabled in either a local or
@@ -227,17 +231,18 @@ const (
 	// TODO: Decide on actual feature bit value.
 	ScriptEnforcedLeaseOptional FeatureBit = 2023
 
-	// maxAllowedSize is a maximum allowed size of feature vector.
+	// MaxBolt11Feature is the maximum feature bit value allowed in bolt 11
+	// invoices.
 	//
-	// NOTE: Within the protocol, the maximum allowed message size is 65535
+	// The base 32 encoded tagged fields in invoices are limited to 10 bits
-	// bytes for all messages. Accounting for the overhead within the feature
+	// to express the length of the field's data.
-	// message to signal the type of message, that leaves us with 65533 bytes
+	//nolint:lll
-	// for the init message itself.  Next, we reserve 4 bytes to encode the
+	// See: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md#tagged-fields
-	// lengths of both the local and global feature vectors, so 65529 bytes
+	//
-	// for the local and global features. Knocking off one byte for the sake
+	// With a maximum length field of 1023 (2^10 -1) and 5 bit encoding,
-	// of the calculation, that leads us to 32764 bytes for each feature
+	// the highest feature bit that can be expressed is:
-	// vector, or 131056 different features.
+	// 1023 * 5 - 1 = 5114.
-	maxAllowedSize = 32764
+	MaxBolt11Feature = 5114
 )
 
 // IsRequired returns true if the feature bit is even, and false otherwise.
@@ -354,8 +359,12 @@ func (fv *RawFeatureVector) Merge(other *RawFeatureVector) error {
 // new feature vector provided, checking that it does not alter any of the
 // "standard" features that are defined by LND. The new feature vector should
 // be inclusive of all features in the original vector that it still wants to
-// advertise, setting and unsetting updates as desired.
+// advertise, setting and unsetting updates as desired. Features in the vector
-func (fv *RawFeatureVector) ValidateUpdate(other *RawFeatureVector) error {
+// are also checked against a maximum inclusive value, as feature vectors in
+// different contexts have different maximum values.
+func (fv *RawFeatureVector) ValidateUpdate(other *RawFeatureVector,
+	maximumValue FeatureBit) error {
+
 	// Run through the new set of features and check that we're not adding
 	// any feature bits that are defined but not set in LND.
 	for feature := range other.features {
@@ -363,6 +372,12 @@ func (fv *RawFeatureVector) ValidateUpdate(other *RawFeatureVector) error {
 			continue
 		}
 
+		if feature > maximumValue {
+			return fmt.Errorf("can't set feature bit %d: %w %v",
+				feature, ErrFeatureBitMaximum,
+				maximumValue)
+		}
+
 		if name, known := Features[feature]; known {
 			return fmt.Errorf("can't set feature "+
 				"bit %d (%v): %w", feature, name,

lnwire/features_test.go

@@ -2,6 +2,7 @@ package lnwire
 
 import (
 	"bytes"
+	"math"
 	"reflect"
 	"sort"
 	"testing"
@@ -423,6 +424,7 @@ func TestValidateUpdate(t *testing.T) {
 		name            string
 		currentFeatures []FeatureBit
 		newFeatures     []FeatureBit
+		maximumValue    FeatureBit
 		err             error
 	}{
 		{
@@ -479,6 +481,30 @@ func TestValidateUpdate(t *testing.T) {
 			},
 			err: nil,
 		},
+		{
+			name: "at allowed maximum",
+			currentFeatures: []FeatureBit{
+				StaticRemoteKeyOptional,
+			},
+			newFeatures: []FeatureBit{
+				StaticRemoteKeyOptional,
+				100,
+			},
+			maximumValue: 100,
+			err:          nil,
+		},
+		{
+			name: "above allowed maximum",
+			currentFeatures: []FeatureBit{
+				StaticRemoteKeyOptional,
+			},
+			newFeatures: []FeatureBit{
+				StaticRemoteKeyOptional,
+				101,
+			},
+			maximumValue: 100,
+			err:          ErrFeatureBitMaximum,
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -491,7 +517,13 @@ func TestValidateUpdate(t *testing.T) {
 			)
 			newFV := NewRawFeatureVector(testCase.newFeatures...)
 
-			err := currentFV.ValidateUpdate(newFV)
+			// Set maximum value if not populated in the test case.
+			maximumValue := testCase.maximumValue
+			if testCase.maximumValue == 0 {
+				maximumValue = math.MaxUint16
+			}
+
+			err := currentFV.ValidateUpdate(newFV, maximumValue)
 			require.ErrorIs(t, err, testCase.err)
 		})
 	}