From 3835b903a9b7ee66965c91252f2ba2f7a31d4c5b Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Mon, 19 Dec 2022 10:51:46 +0100 Subject: [PATCH] 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 | 4 ++ lnwire/onion_error.go | 21 +++++++---- lnwire/onion_error_test.go | 44 ++++++++++++++++++++-- 3 files changed, 59 insertions(+), 10 deletions(-) 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")