Merge pull request #7262 from bottlepay/fix-failure-tlv

lnwire: limit channel update reader

docs/release-notes/release-notes-0.16.0.md

@@ -184,6 +184,10 @@ certain large transactions](https://github.com/lightningnetwork/lnd/pull/7100).
 
 * [Prevent nil pointer dereference during funding manager 
   test](https://github.com/lightningnetwork/lnd/pull/7268)
+  
+* Fixed a [failure message parsing bug](https://github.com/lightningnetwork/lnd/pull/7262)
+  that caused additional failure message data to be interpreted as being part of
+  a channel update.
 
 ## `lncli`
 

lnwire/onion_error.go

@@ -596,9 +596,16 @@ func (f *FailInvalidOnionKey) Error() string {
 // attempt to parse these messages in order to retain compatibility. If we're
 // unable to pull out a fully valid version, then we'll fall back to the
 // regular parsing mechanism which includes the length prefix an NO type byte.
-func parseChannelUpdateCompatabilityMode(r *bufio.Reader,
+func parseChannelUpdateCompatabilityMode(reader io.Reader, length uint16,
 	chanUpdate *ChannelUpdate, pver uint32) error {
 
+	// Instantiate a LimitReader because there may be additional data
+	// present after the channel update. Without limiting the stream, the
+	// additional data would be interpreted as channel update tlv data.
+	limitReader := io.LimitReader(reader, int64(length))
+
+	r := bufio.NewReader(limitReader)
+
 	// We'll peek out two bytes from the buffer without advancing the
 	// buffer so we can decide how to parse the remainder of it.
 	maybeTypeBytes, err := r.Peek(2)
@@ -677,7 +684,7 @@ func (f *FailTemporaryChannelFailure) Decode(r io.Reader, pver uint32) error {
 	if length != 0 {
 		f.Update = &ChannelUpdate{}
 		return parseChannelUpdateCompatabilityMode(
-			bufio.NewReader(r), f.Update, pver,
+			r, length, f.Update, pver,
 		)
 	}
 
@@ -761,7 +768,7 @@ func (f *FailAmountBelowMinimum) Decode(r io.Reader, pver uint32) error {
 
 	f.Update = ChannelUpdate{}
 	return parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(r), &f.Update, pver,
+		r, length, &f.Update, pver,
 	)
 }
 
@@ -829,7 +836,7 @@ func (f *FailFeeInsufficient) Decode(r io.Reader, pver uint32) error {
 
 	f.Update = ChannelUpdate{}
 	return parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(r), &f.Update, pver,
+		r, length, &f.Update, pver,
 	)
 }
 
@@ -897,7 +904,7 @@ func (f *FailIncorrectCltvExpiry) Decode(r io.Reader, pver uint32) error {
 
 	f.Update = ChannelUpdate{}
 	return parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(r), &f.Update, pver,
+		r, length, &f.Update, pver,
 	)
 }
 
@@ -954,7 +961,7 @@ func (f *FailExpiryTooSoon) Decode(r io.Reader, pver uint32) error {
 
 	f.Update = ChannelUpdate{}
 	return parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(r), &f.Update, pver,
+		r, length, &f.Update, pver,
 	)
 }
 
@@ -1018,7 +1025,7 @@ func (f *FailChannelDisabled) Decode(r io.Reader, pver uint32) error {
 
 	f.Update = ChannelUpdate{}
 	return parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(r), &f.Update, pver,
+		r, length, &f.Update, pver,
 	)
 }
 

lnwire/onion_error_test.go

@@ -1,9 +1,9 @@
 package lnwire
 
 import (
-	"bufio"
 	"bytes"
 	"encoding/binary"
+	"encoding/hex"
 	"io"
 	"reflect"
 	"testing"
@@ -82,6 +82,44 @@ func TestEncodeDecodeCode(t *testing.T) {
 	}
 }
 
+// TestEncodeDecodeTlv tests the ability of onion errors to be properly encoded
+// and decoded with tlv data present.
+func TestEncodeDecodeTlv(t *testing.T) {
+	t.Parallel()
+
+	for _, testFailure := range onionFailures {
+		testFailure := testFailure
+		code := testFailure.Code().String()
+
+		t.Run(code, func(t *testing.T) {
+			t.Parallel()
+
+			testEncodeDecodeTlv(t, testFailure)
+		})
+	}
+}
+
+var testTlv, _ = hex.DecodeString("fd023104deadbeef")
+
+func testEncodeDecodeTlv(t *testing.T, testFailure FailureMessage) { //nolint: lll,thelper
+	var failureMessageBuffer bytes.Buffer
+
+	err := EncodeFailureMessage(&failureMessageBuffer, testFailure, 0)
+	require.NoError(t, err)
+
+	failureMessageBuffer.Write(testTlv)
+
+	failure, err := DecodeFailureMessage(&failureMessageBuffer, 0)
+	require.NoError(t, err)
+
+	// FailIncorrectDetails already reads tlv data. Adapt the expected data.
+	if incorrectDetails, ok := testFailure.(*FailIncorrectDetails); ok {
+		incorrectDetails.extraOpaqueData = testTlv
+	}
+
+	require.Equal(t, testFailure, failure)
+}
+
 // TestChannelUpdateCompatabilityParsing tests that we're able to properly read
 // out channel update messages encoded in an onion error payload that was
 // written in the legacy (type prefixed) format.
@@ -100,7 +138,7 @@ func TestChannelUpdateCompatabilityParsing(t *testing.T) {
 	// encoded channel update message.
 	var newChanUpdate ChannelUpdate
 	err := parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(&b), &newChanUpdate, 0,
+		&b, uint16(b.Len()), &newChanUpdate, 0,
 	)
 	require.NoError(t, err, "unable to parse channel update")
 
@@ -127,7 +165,7 @@ func TestChannelUpdateCompatabilityParsing(t *testing.T) {
 	// message even with the extra two bytes.
 	var newChanUpdate2 ChannelUpdate
 	err = parseChannelUpdateCompatabilityMode(
-		bufio.NewReader(&b), &newChanUpdate2, 0,
+		&b, uint16(b.Len()), &newChanUpdate2, 0,
 	)
 	require.NoError(t, err, "unable to parse channel update")