diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index ac6f6c05c..49e51052c 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/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` diff --git a/lnwire/onion_error.go b/lnwire/onion_error.go index 623ca0bf4..e6ae7437f 100644 --- a/lnwire/onion_error.go +++ b/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, ) } diff --git a/lnwire/onion_error_test.go b/lnwire/onion_error_test.go index 27bba342e..96a088065 100644 --- a/lnwire/onion_error_test.go +++ b/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")