parseinvoice accepts BOLT 11 invoice with non-zero bech32 padding bits

[NishantBansal2003]

eclair-cli parseinvoice silently accepts a BOLT 11 invoice containing non-zero bech32 padding bits, which is explicitly invalid per BIP-173. It also silently rewrites the serialized field in its response to a corrected version of the invoice, hiding the fact that the input was malformed.

Spec References

• BOLT 11:
  "MUST parse the address as Bech32, as specified in BIP-0173"

• BIP-173:
  "Any incomplete group at the end MUST be 4 bits or less, MUST be all zeroes, and is discarded."

In BOLT 11, I see there is a requirement for the writer: "MUST pad field data to a multiple of 5 bits, using 0s."
However, I don’t see any requirement for the reader in above case during parsing, So, I think this should also be made explicit in the spec?

Inputs

• Invoice 1 (invalid - non-zero padding): lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqaaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqqjjc7mu

{
  "prefix": "lnbc",
  "timestamp": 524288,
  "nodeId": "02d1840657fa8d4c0fa9ce7ef698bebb4a227cc2893a0dd7b871a2fe7c5f3a85fe",
  "serialized": "lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqsaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqq7hegzf",
  "description": "",
  "paymentHash": "a1294a5280000007301a9c00000c00000000000000005004000000037fff0000",
  "features": {
    "activated": {},
    "unknown": []
  },
  "routingInfo": []
}

• Invoice 2 (valid - zero padding): lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqsaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqq7hegzf

{
  "prefix": "lnbc",
  "timestamp": 524288,
  "nodeId": "036fc69aed4740b8d060a99047e080c699ad803381bcfa2f19be91fcc8c0b4fb1c",
  "serialized": "lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqsaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqq7hegzf",
  "description": "",
  "paymentHash": "a1294a5280000007301a9c00000c00000000000000005004000000037fff0000",
  "features": {
    "activated": {},
    "unknown": []
  },
  "routingInfo": []
}

FYI

CLN

$ lightning-cli decode lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqaaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqqjjc7mu
{
   "code": -32602,
   "message": "string: non-zero trailing bits: invalid token '\"lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqaaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqqjjc7mu\"'"
}

$ lightning-cli decode lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqsaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqq7hegzf
{
   "type": "bolt11 invoice",
   "currency": "bc",
   "created_at": 524288,
   "expiry": 3600,
   "payee": "036fc69aed4740b8d060a99047e080c699ad803381bcfa2f19be91fcc8c0b4fb1c",
   "description": "",
   "min_final_cltv_expiry": 18,
   "payment_secret": "a5294a5294a5294a5294a50210842108421ef7bdb77bdef7bfffffff881def41",
   "features": "",
   "extra": [
      {
         "tag": "q",
         "data": ""
      },
      {
         "tag": "4",
         "data": "pq9pqqq"
      },
      {
         "tag": "q",
         "data": "pqq9q8p"
      },
      {
         "tag": "a",
         "data": ""
      },
      {
         "tag": "q",
         "data": ""
      },
      {
         "tag": "a",
         "data": ""
      },
      {
         "tag": "q",
         "data": ""
      },
      {
         "tag": "q",
         "data": "jqfq5jpppp"
      },
      {
         "tag": "a",
         "data": ""
      },
      {
         "tag": "q",
         "data": ""
      },
      {
         "tag": "a",
         "data": ""
      },
      {
         "tag": "q",
         "data": ""
      },
      {
         "tag": "q",
         "data": "jqfq5jpppp"
      },
      {
         "tag": "q",
         "data": ""
      }
   ],
   "payment_hash": "a1294a5280000007301a9c00000c00000000000000005004000000037fff0000",
   "signature": "3041021c00c000000000000002819fffffffffff7087ff813fffffffffffffff022100ffffe0ffff4903e0677fffe7fffffe0000000001fffffffd294a5294a7fffffc",
   "valid": true
}

LND

$ lncli decodepayreq lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqaaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqqjjc7mu
{
    "error": "invalid incomplete group"
}

$ lncli decodepayreq lnbc1qqqsqqqqqq4q8pq9pqqqqq8pqq9q8pdqqaqqqqqaqqqqqqq2jqfq5jppppsp5555555555555555555ppppppppppaaaakaaaaaallllllzqaaaqsaqqqqqaqqqqqqq2jqfq5jpppppp55y55555qqqqqwvq6nsqqqrqqqqqqqqqqqqq9qpqqqqqqxlllqqqqqqqqqqqqqqqcqqqqqqqqqqq9qvlllllllllwzrllqfllllllllllllllllqlll5jqlqvalllelllllqqqqqqqqllllll55555555lllllqq7hegzf
{
    "destination": "036fc69aed4740b8d060a99047e080c699ad803381bcfa2f19be91fcc8c0b4fb1c",
    "payment_hash": "a1294a5280000007301a9c00000c00000000000000005004000000037fff0000",
    "num_satoshis": "0",
    "timestamp": "524288",
    "expiry": "3600",
    "description": "",
    "description_hash": "",
    "fallback_addr": "",
    "cltv_expiry": "18",
    "route_hints": [],
    "payment_addr": "a5294a5294a5294a5294a50210842108421ef7bdb77bdef7bfffffff881def41",
    "num_msat": "0",
    "features": {},
    "blinded_paths": []
}

PS: This was found during fuzzing while I was adding BOLT 11 invoice deserialization fuzz tests, and it serves as a reminder of the importance of having a good fuzz test suite for Eclair.

[t-bast]

Bolt11 encoding really is a mess 😅 . There are a lot of known divergences between implementations in what they accept and reject (the 5-bit encoding was not very a good idea).

I'm not sure this really matters though? How is that an issue in practice? The only important facts to check are that:

• implementations generate valid invoices (there's no reason anyone would generate an invoice with non-0 padding bytes)
• implementations don't reject valid invoices
• implementations don't crash on malformed/malicious invoices

I don't think this specific issue creates any real concern, it is nice to fix it, but low priority.

As I highlighted earlier, I think codecs are not very interesting fuzzing targets: it is very easy to write fuzz tests for them so it's tempting to test everything, but IMO it's mostly a waste of computing resources (it won't find an important bug). It is harder, but way more useful to find ways to fuzz the channel state machine to find cases where malicious spends aren't detected, or where the channel state machine repeatedly crashes, which would allow funds to be stolen.

[NishantBansal2003]

• implementations generate valid invoices (there's no reason anyone would generate an invoice with non-0 padding bytes)

It may not necessarily be the case that the invoice I receive always comes from a well-behaved implementation, right? An attacker could potentially provide an arbitrary invoice if there is any way to exploit it (even though this particular issue does not lead to a crash).

If the implementation can ensure that an invoice is valid at the time of generation, i think it make sense to enforce similar validation when reading the invoice (at least during parsing)

I don't think this specific issue creates any real concern, it is nice to fix it, but low priority.

I agree that this issue does not have much impact on users. However, if this requirement is part of the spec (and other LN implementations are already following it), i think it make sense to not left it as-is

As I highlighted earlier, I think codecs are not very interesting fuzzing targets: it is very easy to write fuzz tests for them so it's tempting to test everything, but IMO it's mostly a waste of computing resources (it won't find an important bug).

Codec fuzz tests are relatively easy to write, but I think they can still provide value. For eg: LND previously had a DoS incident that was discovered through relatively simple codec fuzzing See
So I think, it might still be worthwhile to include them rather than leaving them solely because they are simple. Also, if fuzzing results become stagnant over time, we can always choose to stop or reduce fuzzing for those components. Selective fuzzing based on the maturity and effectiveness of tests could be a reasonable approach to follow.

It is harder, but way more useful to find ways to fuzz the channel state machine to find cases where malicious spends aren't detected, or where the channel state machine repeatedly crashes, which would allow funds to be stolen.

State machine fuzzing for eclair is already on my TODO list. Some of the important states can be easily fuzzed usin smite, and for the remaining ones, I will add fuzz tests directly in eclair