lnwire: limit channel update reader

Fixes a bug where channel update data is read until the end of the stream
rather than stopping after the specified length. This is problematic
when failure message tlv data is present, because this data is interpreted
as channel update tlv data.

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

@@ -185,6 +185,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`
 
 * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice

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")