diff --git a/docs/release-notes/release-notes-0.20.0.md b/docs/release-notes/release-notes-0.20.0.md index af512597e..a6bc8d2e6 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. diff --git a/zpay32/decode.go b/zpay32/decode.go index 76c2c1ecf..f97c77838 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,21 @@ 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") + + // 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 // Decode function. type DecodeOption func(*decodeOptions) @@ -446,6 +462,10 @@ func parseDescription(data []byte) (*string, error) { return nil, err } + if !utf8.Valid(base256Data) { + return nil, ErrInvalidUTF8Description + } + description := string(base256Data) return &description, nil @@ -565,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 diff --git a/zpay32/invoice_internal_test.go b/zpay32/invoice_internal_test.go index 35091d6a6..0a41b4c4d 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\"", @@ -685,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, @@ -704,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, @@ -720,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) 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"