[bug]: BOLT 11 Compliance: Invoice `r` field don't reject empty routing hints

[erickcestari]

Background

After doing some differential fuzzing between CLN, rust-lightning and LND using bitcoinfuzz I noticed that LND currently accepts bolt11 invoices with empty routing hints in the r field, while CLN and rust-lightning rejects. This violates BOLT 11 specification requirements.

BOLT 11 Requirements:

• r field "MUST contain one or more ordered entries, indicating the forward route from a public node to the final destination"
• Each hop must contain exactly 51 bytes (pubkey: 33B, short_channel_id: 8B, fee_base_msat: 4B, fee_proportional_millionths: 4B, cltv_expiry_delta: 2B)

Current Behavior: The parseRouteHint function returns an empty slice without error when base256Data length is 0, since 0 % 51 = 0 passes the modulo check and the parsing loop never executes.

Expected Behavior: Reject invoices with empty r fields.

Impact:

• Spec non-compliance
• Inconsistent behavior between Lightning implementations

Proposed Fix: Add empty data check in parseRouteHint after bech32 conversion:

// Check for empty route hint
if len(base256Data) == 0 {
    return nil, fmt.Errorf("r field contains no hop data")
}

Example invoice:

lnbc1p5q54jjpp5fe0dhqdt4m97psq0fv3wjlk95cclnatvuvq49xtnc8rzrp0dysusdqqcqzzsxqrrs0fppqy6uew5229e67r9xzzm9mjyfwseclstdgsp5rnanj9x5rnanj9xnq28hhgd6c7yxlmh6lta047h6lqqqqqqqqqqqrqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq6qqqqqqqqqqqqqqqqqqq9kvnknh7ug5mttnqqqqqqqqq8849gwfhvnp9rqpe0cy97

[MPins]

@erickcestari and @saubyk I was able to reproduce the behavior (IMO not an issue) locally using my regtest environment by modifying a unit test, as shown below.

To simulate the problem, I crafted an invoice that includes an empty route hints:

lnbcrt11p59f5ljpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqh23jhxar9yp8x7gzjda6hgegrqqfjsm9c8yvkp73csuzsewd4ddh9tgukaxcv20pnyvxvhtu8rzqr5rfhtkm5lwgpfqs8r0v9nyj9rmmh2qkmagx48eg5mjldu8dvtgpfcqjtu5e0

We can see using the command decodepayreq that the invoice include an empty route hints:

alice:/# lncli decodepayreq lnbcrt11p59f5ljpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqh23jhxar9yp8x7gzjda6hgegrqqfjsm9c8yvkp73csuzsewd4ddh9tgukaxcv20pnyvxvhtu8rzqr5rfhtkm5lwgpfqs8r0v9nyj9rmmh2qkmagx48eg5mjldu8dvtgpfcqjtu5e0
{
    "destination": "03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad",
    "payment_hash": "0001020304050607080900010203040506070809000102030405060708090102",
    "num_satoshis": "100000000",
    "timestamp": "1750389746",
    "expiry": "3600",
    "description": "Teste No Route",
    "description_hash": "",
    "fallback_addr": "",
    "cltv_expiry": "18",
    "route_hints": [
        {
            "hop_hints": []
        }
    ],
    "payment_addr": "0001020304050607080900010203040506070809000102030405060708090102",
    "num_msat": "100000000000",
    "features": {},
    "blinded_paths": []
}

We can see using the command payinvoice that the empty route hints is accepted:

alice:/# lncli payinvoice lnbcrt11p59f5ljpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqh23jhxar9yp8x7gzjda6hgegrqqfjsm9c8yvkp73csuzsewd4ddh9tgukaxcv20pnyvxvhtu8rzqr5rfhtkm5lwgpfqs8r0v9nyj9rmmh2qkmagx48eg5mjldu8dvtgpfcqjtu5e0
Payment hash: 0001020304050607080900010203040506070809000102030405060708090102
Description: Teste No Route
Amount (in satoshis): 100000000
Fee limit (in satoshis): 5000000
Destination: 03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad
Confirm payment (yes/no): yes
+------------+--------------+--------------+--------------+-----+----------+----------+-------+
| HTLC_STATE | ATTEMPT_TIME | RESOLVE_TIME | RECEIVER_AMT | FEE | TIMELOCK | CHAN_OUT | ROUTE |
+------------+--------------+--------------+--------------+-----+----------+----------+-------+
+------------+--------------+--------------+--------------+-----+----------+----------+-------+
Amount + fee:   0 + 0 sat
Payment hash:   0001020304050607080900010203040506070809000102030405060708090102
Payment status: FAILED, reason: FAILURE_REASON_NO_ROUTE

To catch the issue during unit testing, I added the invoice above to the TestExtractIntentFromSendRequest unit test and introduced a test case that uses the invoice, marking it as valid.

// TestExtractIntentFromSendRequest verifies that extractIntentFromSendRequest
// correctly translates a SendPaymentRequest from an RPC client into a
// LightningPayment intent.
func TestExtractIntentFromSendRequest(t *testing.T) {
.
.
.
	const paymentReqEmptyRouteHints = "lnbcrt11p59f5ljpp5qqqsyqcyq5rqwzqf" +
		"qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5qqqsyqcyq5rqwzqfqqqsy" +
		"qcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqh23jhxar9yp8x7gzjda6hgegrqq" +
		"fjsm9c8yvkp73csuzsewd4ddh9tgukaxcv20pnyvxvhtu8rzqr5rfhtkm5lw" +
		"gpfqs8r0v9nyj9rmmh2qkmagx48eg5mjldu8dvtgpfcqjtu5e0"
.
.
.
	testCases := []extractIntentTestCase{
.
.
.
		{
			name: "Invoice with Empty Route Hints",
			backend: &RouterBackend{
				ShouldSetExpEndorsement: func() bool {
					return false
				},
				ActiveNetParams:  &chaincfg.RegressionNetParams,
				MaxTotalTimelock: 1000,
				Clock:            mockClock,
			},
			sendReq: &SendPaymentRequest{
				PaymentRequest: paymentReqEmptyRouteHints,
				CltvLimit:      22,
			},
			valid: true,
		},
.
.
.
}

The result shows that the testcase was executed with success:

Running tool: /usr/local/go/bin/go test -timeout 30s -tags autopilotrpc chainrpc dev invoicesrpc neutrinorpc peersrpc signrpc walletrpc watchtowerrpc -run ^TestExtractIntentFromSendRequest$ github.com/lightningnetwork/lnd/lnrpc/routerrpc

=== RUN   TestExtractIntentFromSendRequest
.
.
.
--- PASS: TestExtractIntentFromSendRequest/Invoice_with_Empty_Route_Hints (0.00s)
.
.
.
PASS
ok      github.com/lightningnetwork/lnd/lnrpc/routerrpc 0.021s

[MPins]

@erickcestari I see your point, but IMO it's not an issue because the BOLT requirements mentioned above are writer-side constraints.

[MPins]

@erickcestari and @saubyk this issue might be closed so.

[erickcestari]

@erickcestari I see your point, but IMO it's not an issue because the BOLT requirements mentioned above are writer-side constraints.

Sorry for the late response. I was reviewing the spec again and I should have added this in the issue:

• r (3): data_length variable. One or more entries containing extra routing information for a private route; there may be more than one r field
  • pubkey (264 bits)
  • short_channel_id (64 bits)
  • fee_base_msat (32 bits, big-endian)
  • fee_proportional_millionths (32 bits, big-endian)
  • cltv_expiry_delta (16 bits, big-endian)

I believe this falls into the same category as #9904 - while the reader-side rejection isn't explicitly stated, the tagged field definition specifies that it should contain "One or more entries." Therefore, any field without entries should be rejected, as it violates the data type specification.

[MPins]

@erickcestari I see your point, but IMO it's not an issue because the BOLT requirements mentioned above are writer-side constraints.

Sorry for the late response. I was reviewing the spec again and I should have added this in the issue:

• r (3): data_length variable. One or more entries containing extra routing information for a private route; there may be more than one r field

  • pubkey (264 bits)
  • short_channel_id (64 bits)
  • fee_base_msat (32 bits, big-endian)
  • fee_proportional_millionths (32 bits, big-endian)
  • cltv_expiry_delta (16 bits, big-endian)

I believe this falls into the same category as #9904 - while the reader-side rejection isn't explicitly stated, the tagged field definition specifies that it should contain "One or more entries." Therefore, any field without entries should be rejected, as it violates the data type specification.

This case is a bit different. Unlike #9904, where the specification requires the reader to fail if the field is missing, here there is no such requirement.

Would be good to hear others' thoughts on this as well.