From 2b1a89fbafa81d2d9e84c4b737c46a38a64db539 Mon Sep 17 00:00:00 2001 From: MPins Date: Tue, 24 Jun 2025 16:28:43 -0300 Subject: [PATCH 1/5] zpay32: validate UTF-8 parsing field description --- zpay32/decode.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/zpay32/decode.go b/zpay32/decode.go index 76c2c1ecf..9a2277f46 100644 --- a/zpay32/decode.go +++ b/zpay32/decode.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" "time" + "unicode/utf8" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcec/v2/ecdsa" @@ -18,6 +19,12 @@ import ( "github.com/lightningnetwork/lnd/lnwire" ) +var ( + // ErrInvalidUTF8Description is returned if the invoice description is + // not valid UTF-8. + ErrInvalidUTF8Description = errors.New("description is not valid UTF-8") +) + // DecodeOption is a type that can be used to supply functional options to the // Decode function. type DecodeOption func(*decodeOptions) @@ -446,6 +453,10 @@ func parseDescription(data []byte) (*string, error) { return nil, err } + if !utf8.Valid(base256Data) { + return nil, ErrInvalidUTF8Description + } + description := string(base256Data) return &description, nil From b8acca2c892d1961e65a6b58dcb925c41cc5c3d0 Mon Sep 17 00:00:00 2001 From: MPins Date: Tue, 24 Jun 2025 16:29:21 -0300 Subject: [PATCH 2/5] zpay32: test case for parsing non UTF-8 description --- zpay32/invoice_internal_test.go | 18 +++++++++++++++--- zpay32/invoice_test.go | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/zpay32/invoice_internal_test.go b/zpay32/invoice_internal_test.go index 35091d6a6..a9fed9bc8 100644 --- a/zpay32/invoice_internal_test.go +++ b/zpay32/invoice_internal_test.go @@ -369,14 +369,25 @@ func TestParse32Bytes(t *testing.T) { func TestParseDescription(t *testing.T) { t.Parallel() + testNonUTF8StrData, _ := bech32.ConvertBits( + []byte(testNonUTF8Str), 8, 5, true, + ) + testCupOfCoffeeData, _ := bech32.ConvertBits([]byte(testCupOfCoffee), 8, 5, true) testPleaseConsiderData, _ := bech32.ConvertBits([]byte(testPleaseConsider), 8, 5, true) tests := []struct { - data []byte - valid bool - result *string + data []byte + valid bool + result *string + expectedErr error }{ + { + data: testNonUTF8StrData, + valid: false, + expectedErr: ErrInvalidUTF8Description, + result: nil, + }, { data: []byte{}, valid: true, @@ -400,6 +411,7 @@ func TestParseDescription(t *testing.T) { t.Errorf("description decoding test %d failed: %v", i, err) return } + require.ErrorIs(t, err, test.expectedErr) if test.valid && !reflect.DeepEqual(description, test.result) { t.Fatalf("test %d failed decoding description: "+ "expected \"%s\", got \"%s\"", diff --git a/zpay32/invoice_test.go b/zpay32/invoice_test.go index 55718007d..4cb1a6170 100644 --- a/zpay32/invoice_test.go +++ b/zpay32/invoice_test.go @@ -50,6 +50,7 @@ var ( 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, } + testNonUTF8Str = "1 cup coffee\xff\xfe\xfd" testEmptyString = "" testCupOfCoffee = "1 cup coffee" testCoffeeBeans = "coffee beans" From 16187f047ac6a7bac342d9f709f0f0ae61fed4a1 Mon Sep 17 00:00:00 2001 From: MPins Date: Tue, 24 Jun 2025 17:52:30 -0300 Subject: [PATCH 3/5] zpay32: validate empty routing hints when parsing r fields --- zpay32/decode.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/zpay32/decode.go b/zpay32/decode.go index 9a2277f46..f97c77838 100644 --- a/zpay32/decode.go +++ b/zpay32/decode.go @@ -23,6 +23,15 @@ var ( // ErrInvalidUTF8Description is returned if the invoice description is // not valid UTF-8. ErrInvalidUTF8Description = errors.New("description is not valid UTF-8") + + // ErrLengthNotMultipleOfHopHintLength is returned if the length of the + // route hint data is not a multiple of the hop hint length. + ErrLengthNotMultipleOfHopHint = errors.New("length is not a multiple " + + "of hop hint length") + + // ErrEmptyRouteHint is returned if the route hint field contains no hop + // data. + ErrEmptyRouteHint = errors.New("route hint field contains no hop data") ) // DecodeOption is a type that can be used to supply functional options to the @@ -576,8 +585,12 @@ func parseRouteHint(data []byte) ([]HopHint, error) { // Check that base256Data is a multiple of hopHintLen. if len(base256Data)%hopHintLen != 0 { - return nil, fmt.Errorf("expected length multiple of %d bytes, "+ - "got %d", hopHintLen, len(base256Data)) + return nil, ErrLengthNotMultipleOfHopHint + } + + // Check for empty route hint + if len(base256Data) == 0 { + return nil, ErrEmptyRouteHint } var routeHint []HopHint From e61d971ce48c8506584fc4124f2be1967a3a24fe Mon Sep 17 00:00:00 2001 From: MPins Date: Tue, 24 Jun 2025 17:53:11 -0300 Subject: [PATCH 4/5] zpay32: test case for empty routing hints on r fields --- zpay32/invoice_internal_test.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/zpay32/invoice_internal_test.go b/zpay32/invoice_internal_test.go index a9fed9bc8..0a41b4c4d 100644 --- a/zpay32/invoice_internal_test.go +++ b/zpay32/invoice_internal_test.go @@ -697,18 +697,22 @@ func TestParseRouteHint(t *testing.T) { testDoubleHopData, _ = bech32.ConvertBits(testDoubleHopData, 8, 5, true) tests := []struct { - data []byte - valid bool - result []HopHint + data []byte + valid bool + result []HopHint + expectedErr error }{ { - data: []byte{0x0, 0x0, 0x0, 0x0}, - valid: false, // data too short, not multiple of 51 bytes + data: []byte{0x0, 0x0, 0x0, 0x0}, + // data too short, not multiple of 51 bytes + valid: false, + expectedErr: ErrLengthNotMultipleOfHopHint, }, { - data: []byte{}, - valid: true, - result: []HopHint{}, + data: []byte{}, + valid: false, + result: []HopHint{}, + expectedErr: ErrEmptyRouteHint, }, { data: testSingleHopData, @@ -716,8 +720,11 @@ func TestParseRouteHint(t *testing.T) { result: testSingleHop, }, { - data: append(testSingleHopData, 0x0), - valid: false, // data too long, not multiple of 51 bytes + data: append(testSingleHopData, + []byte{0x0, 0x0}...), + // data too long, not multiple of 51 bytes + valid: false, + expectedErr: ErrLengthNotMultipleOfHopHint, }, { data: testDoubleHopData, @@ -732,6 +739,7 @@ func TestParseRouteHint(t *testing.T) { t.Errorf("routing info decoding test %d failed: %v", i, err) return } + require.ErrorIs(t, err, test.expectedErr) if test.valid { if err := compareRouteHints(test.result, routeHint); err != nil { t.Fatalf("test %d failed decoding routing info: %v", i, err) From 2815d4922c9b19fc17076c75519a1635681b07a3 Mon Sep 17 00:00:00 2001 From: MPins Date: Tue, 24 Jun 2025 18:02:05 -0300 Subject: [PATCH 5/5] doc: release-notes-0.20.0 --- docs/release-notes/release-notes-0.20.0.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index 1485fd996..0070219b6 100644 --- a/docs/release-notes/release-notes-0.20.0.md +++ b/docs/release-notes/release-notes-0.20.0.md @@ -29,6 +29,10 @@ - Fixed [shutdown deadlock](https://github.com/lightningnetwork/lnd/pull/10042) when we fail starting up LND before we startup the chanbackup sub-server. +- Fixed BOLT-11 invoice parsing behavior: [now errors]( + https://github.com/lightningnetwork/lnd/pull/9993) are returned when receiving + empty route hints or a non-UTF-8-encoded description. + - [Fixed](https://github.com/lightningnetwork/lnd/pull/10027) an issue where known TLV fields were incorrectly encoded into the `ExtraData` field of messages in the dynamic commitment set.