From b654a3b7437ff233030f333f0c6cbadb1069d877 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 10:19:53 +0200 Subject: [PATCH 1/9] bolt11: catch tag padding and encoding errors, ecc errors, and raise as BOLT11DecodeException also make fallback address parsing more strict. This implements tightened rules as described in https://github.com/lightning/bolts/pull/1243 - MUST include exactly one p field - MUST include exactly one s field - MUST include either exactly one d or exactly one h field --- electrum/bolt11.py | 163 +++++++++++++++++++++++++++-------------- tests/test_bolt11.py | 171 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 277 insertions(+), 57 deletions(-) diff --git a/electrum/bolt11.py b/electrum/bolt11.py index dcf6e360fa03..e96e03d70c8c 100644 --- a/electrum/bolt11.py +++ b/electrum/bolt11.py @@ -7,13 +7,13 @@ from hashlib import sha256 from binascii import hexlify from decimal import Decimal -from typing import Optional, TYPE_CHECKING, Type, Dict, Any, Sequence, Tuple +from typing import Optional, TYPE_CHECKING, Type, Dict, Any, Sequence, Tuple, List import random import electrum_ecc as ecc from .bitcoin import hash160_to_b58_address, b58_address_to_hash160, TOTAL_COIN_SUPPLY_LIMIT_IN_BTC -from .segwit_addr import bech32_encode, bech32_decode, CHARSET, CHARSET_INVERSE, convertbits +from .segwit_addr import bech32_encode, bech32_decode, CHARSET, CHARSET_INVERSE, convertbits, INVALID_BECH32 from . import segwit_addr from . import constants from .constants import AbstractNet @@ -96,13 +96,24 @@ def encode_fallback_addr(fallback: str, net: Type[AbstractNet]) -> Sequence[int] def parse_fallback_addr(data5: Sequence[int], net: Type[AbstractNet]) -> Optional[str]: + """Returns None if the fallback address cannot be parsed.""" + if not data5: + return None wver = data5[0] - data8 = bytes(convertbits(data5[1:], 5, 8, False)) + data8 = convertbits(data5[1:], 5, 8, False) + if data8 is None: # invalid padding + return None + data8 = bytes(data8) if wver == 17: + if len(data8) != 20: # hash160 + return None addr = hash160_to_b58_address(data8, net.ADDRTYPE_P2PKH) elif wver == 18: + if len(data8) != 20: # hash160 + return None addr = hash160_to_b58_address(data8, net.ADDRTYPE_P2SH) elif wver <= 16: + # note: encode_segwit_address checks the witness program length addr = segwit_addr.encode_segwit_address(net.SEGWIT_HRP, wver, data8) else: return None @@ -255,8 +266,15 @@ def encode_bolt11_invoice(addr: 'BOLT11Addr', privkey) -> str: class BOLT11Addr: - def __init__(self, *, paymenthash: bytes = None, amount=None, net: Type[AbstractNet] = None, tags=None, date=None, - payment_secret: bytes = None): + def __init__( + self, *, + paymenthash: bytes = None, + amount: Optional[Decimal] = None, + net: Type[AbstractNet] = None, + tags: Optional[List[Tuple[str, Any]]] = None, + date: Optional[int] = None, + payment_secret: bytes = None + ): self.date = int(time.time()) if not date else int(date) self.tags = [] if not tags else tags self.unknown_tags = [] @@ -405,16 +423,31 @@ def serialize(self): def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Addr: - """Parses a string into a BOLT11Addr object. - Can raise BOLT11DecodeException or IncompatibleOrInsaneFeatures. + """Parses a bech32 encoded string into a BOLT11Addr object. + Can raise BOLT11DecodeException. """ + + def _convertbits_tag( + tag: str, + data5: Sequence[int], + *args, + length_range: Optional[Tuple[int, int]] = None, + ) -> Sequence[int]: + if length_range is not None and not length_range[0] <= len(data5) <= length_range[1]: + raise BOLT11DecodeException( + f"Invalid data_length for tag '{tag}': {len(data5)} (expected {length_range})") + if (intseq := convertbits(data5, *args)) is None: + raise BOLT11DecodeException(f"Failed to decode tag '{tag}'") + return intseq + if net is None: net = constants.net decoded_bech32 = bech32_decode(invoice, ignore_long_length=True) + if decoded_bech32 is INVALID_BECH32: + raise BOLT11DecodeException("Invalid bech32 checksum") hrp = decoded_bech32.hrp data5 = decoded_bech32.data # "5" as in list of 5-bit integers - if decoded_bech32.encoding is None: - raise BOLT11DecodeException("Bad bech32 checksum") + assert data5 is not None if decoded_bech32.encoding != segwit_addr.Encoding.BECH32: raise BOLT11DecodeException("Bad bech32 encoding: must be using vanilla BECH32") @@ -451,14 +484,19 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add data5_remaining = data5_remaining[7:] while data5_remaining: - tag, tagdata = pull_tagged(data5_remaining) # mutates arg + try: + tag, tagdata = pull_tagged(data5_remaining) # mutates arg + except ValueError as e: + raise BOLT11DecodeException(f"Corrupt tag data: {str(e)}") # BOLT #11: # - # A reader MUST skip over unknown fields, an `f` field with unknown - # `version`, or a `p`, `h`, or `n` field which does not have - # `data_length` 52, 52, or 53 respectively. - data_length = len(tagdata) + # A reader: + # - MUST skip over `f` fields that use an unknown `version`. + # - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) + # does not have the correct length (52, 52, 52, 53). + # note: the fixed-length check is done via _convertbits_tag(length_range=...) below. + # Until bolts#1243 (2025-06) the spec instead required *skipping* such fields if tag == 'r': # BOLT #11: @@ -471,7 +509,7 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add # * `feebase` (32 bits, big-endian) # * `feerate` (32 bits, big-endian) # * `cltv_expiry_delta` (16 bits, big-endian) - tagdata = convertbits(tagdata, 5, 8, False) + tagdata = _convertbits_tag(tag, tagdata, 5, 8, False) if not tagdata: continue route = [] @@ -491,7 +529,7 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add if route: addr.tags.append(('r',route)) elif tag == 't': - tagdata = convertbits(tagdata, 5, 8, False) + tagdata = _convertbits_tag(tag, tagdata, 5, 8, False) if not tagdata: continue route = [] @@ -514,41 +552,41 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add # Incorrect version. addr.unknown_tags.append((tag, tagdata)) continue - elif tag == 'd': - addr.tags.append(('d', bytes(convertbits(tagdata, 5, 8, False)).decode('utf-8'))) - + if addr.get_tag('d') is not None: + raise BOLT11DecodeException("Unexpected multiple 'd' tags") + try: + addr.tags.append(('d', bytes(_convertbits_tag(tag, tagdata, 5, 8, False)).decode('utf-8'))) + except UnicodeDecodeError as e: + raise BOLT11DecodeException(f"Invalid UTF-8 content in invoice: {str(e)}") elif tag == 'h': - if data_length != 52: - addr.unknown_tags.append((tag, tagdata)) - continue - addr.tags.append(('h', bytes(convertbits(tagdata, 5, 8, False)))) - + if addr.get_tag('h') is not None: + raise BOLT11DecodeException("Unexpected multiple 'h' tags") + addr.tags.append( + ('h', bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(52, 52)))) + ) elif tag == 'x': addr.tags.append(('x', int_from_data5(tagdata))) - elif tag == 'p': - if data_length != 52: - addr.unknown_tags.append((tag, tagdata)) - continue - addr.paymenthash = bytes(convertbits(tagdata, 5, 8, False)) - + # MUST include exactly one 'p' field + if addr.paymenthash is not None: + raise BOLT11DecodeException("Unexpected 'p' tag") + addr.paymenthash = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(52, 52))) elif tag == 's': - if data_length != 52: - addr.unknown_tags.append((tag, tagdata)) - continue - addr.payment_secret = bytes(convertbits(tagdata, 5, 8, False)) - + # MUST include exactly one 's' field + if addr.payment_secret is not None: + raise BOLT11DecodeException("Unexpected 's' tag") + addr.payment_secret = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(52, 52))) elif tag == 'n': - if data_length != 53: - addr.unknown_tags.append((tag, tagdata)) + # if a writer offers more than one of any field type, it: + # MUST specify the most-preferred field first, followed by less-preferred fields, in order. + # as we store a single pubkey, we only store the first + if addr.pubkey is not None: continue - pubkeybytes = bytes(convertbits(tagdata, 5, 8, False)) + pubkeybytes = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(53, 53))) addr.pubkey = pubkeybytes - elif tag == 'c': addr.tags.append(('c', int_from_data5(tagdata))) - elif tag == '9': features = int_from_data5(tagdata) addr.tags.append(('9', features)) @@ -560,6 +598,16 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add else: addr.unknown_tags.append((tag, tagdata)) + # MUST include exactly one 'p' field + if addr.paymenthash is None: + raise BOLT11DecodeException("Missing 'p' tag") + # MUST include exactly one 's' field + if addr.payment_secret is None: + raise BOLT11DecodeException("Missing 's' tag") + # MUST include either exactly one d or exactly one h field + if bool(addr.get_tag('d') is not None) + bool(addr.get_tag('h') is not None) != 1: + raise BOLT11DecodeException("Exactly one of 'd' tag or 'h' tag must be present in invoice") + if verbose: print('hex of signature data (32 byte r, 32 byte s): {}' .format(hexlify(sigdecoded[0:64]))) @@ -575,20 +623,25 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add # field specified below). addr.signature = sigdecoded[:65] hrp_hash = sha256(hrp.encode("ascii") + bytes(convertbits(data5, 5, 8, True))).digest() - if addr.pubkey: # Specified by `n` - # BOLT #11: - # - # A reader MUST use the `n` field to validate the signature instead of - # performing signature recovery if a valid `n` field is provided. - if not ecc.ECPubkey(addr.pubkey).ecdsa_verify(sigdecoded[:64], hrp_hash): - raise BOLT11DecodeException("bad signature") - pubkey_copy = addr.pubkey - - class WrappedBytesKey: - serialize = lambda: pubkey_copy - - addr.pubkey = WrappedBytesKey - else: # Recover pubkey from signature. - addr.pubkey = SerializableKey(ecc.ECPubkey.from_ecdsa_sig64(sigdecoded[:64], sigdecoded[64], hrp_hash)) + try: + if addr.pubkey: # Specified by `n` + # BOLT #11: + # + # A reader MUST use the `n` field to validate the signature instead of + # performing signature recovery if a valid `n` field is provided. + if not ecc.ECPubkey(addr.pubkey).ecdsa_verify(sigdecoded[:64], hrp_hash): + raise BOLT11DecodeException("bad signature") + pubkey_copy = addr.pubkey + + class WrappedBytesKey: + serialize = lambda: pubkey_copy + + addr.pubkey = WrappedBytesKey + else: # Recover pubkey from signature. + addr.pubkey = SerializableKey(ecc.ECPubkey.from_ecdsa_sig64(sigdecoded[:64], sigdecoded[64], hrp_hash)) + except Exception as e: + if isinstance(e, BOLT11DecodeException): + raise + raise BOLT11DecodeException(f"Invalid signature: {e}") from e return addr diff --git a/tests/test_bolt11.py b/tests/test_bolt11.py index 01453a87b6cf..fcef42f8e1b1 100644 --- a/tests/test_bolt11.py +++ b/tests/test_bolt11.py @@ -4,8 +4,12 @@ import pprint import unittest -from electrum.bolt11 import shorten_amount, unshorten_amount, BOLT11Addr, encode_bolt11_invoice, decode_bolt11_invoice -from electrum.segwit_addr import bech32_encode, bech32_decode +import electrum_ecc as ecc + +from electrum.bolt11 import (shorten_amount, unshorten_amount, BOLT11Addr, encode_bolt11_invoice, + decode_bolt11_invoice, parse_fallback_addr, int_to_data5, tagged5, tagged8, + BOLT11DecodeException) +from electrum.segwit_addr import bech32_encode, bech32_decode, convertbits from electrum import segwit_addr from electrum.lnutil import UnknownEvenFeatureBits, LnFeatures, IncompatibleLightningFeatures from electrum import constants @@ -138,6 +142,169 @@ def test_n_decoding(self): lnaddr = decode_bolt11_invoice(bech32_encode(segwit_addr.Encoding.BECH32, hrp, data), verbose=True) self.assertEqual(lnaddr.pubkey.serialize(), PUBKEY) + @staticmethod + def _encode_invoice_with_raw_tag(tag, tagdata5, *, net=None, date=1615922274, expiry=None) -> str: + """Builds a correctly signed invoice with one arbitrary (possibly malformed) tagged field.""" + net = net or constants.BitcoinMainnet + hrp = 'ln' + net.BOLT11_HRP + data5 = list(int_to_data5(date, bit_len=35)) + if tag != 'p': + data5 += list(tagged8('p', RHASH)) + if tag != 's': + data5 += list(tagged8('s', PAYMENT_SECRET)) + if tag not in ('d', 'h'): # exactly one of 'd'/'h' must be present + data5 += list(tagged8('d', b'test')) + if expiry is not None: + data5 += list(tagged5('x', int_to_data5(expiry))) + data5 += list(tagged5(tag, list(tagdata5))) + msg32 = sha256(hrp.encode('ascii') + bytes(convertbits(data5, 5, 8))).digest() + sig = ecc.ECPrivkey(PRIVKEY).ecdsa_sign_recoverable(msg32, is_compressed=False) + sig = bytes(sig[1:]) + bytes([sig[0] - 27]) + return bech32_encode(segwit_addr.Encoding.BECH32, hrp, data5 + list(convertbits(sig, 8, 5, False))) + + @staticmethod + def _encode_invoice_with_raw_sig(sig65, *, net=None) -> str: + """Builds an invoice with an arbitrary (possibly invalid) 65-byte signature. + Note: no 'n' field, so decoding goes through pubkey recovery.""" + net = net or constants.BitcoinMainnet + hrp = 'ln' + net.BOLT11_HRP + data5 = list(int_to_data5(1615922274, bit_len=35)) + data5 += list(tagged8('p', RHASH)) + data5 += list(tagged8('d', b'test')) + return bech32_encode(segwit_addr.Encoding.BECH32, hrp, data5 + list(convertbits(sig65, 8, 5, False))) + + def test_parse_fallback_addr(self): + net = constants.BitcoinMainnet + + def parse(wver, data8): + return parse_fallback_addr([wver] + list(convertbits(data8, 8, 5)), net) + + # p2pkh/p2sh: the payload must be a hash160 + self.assertEqual('1111111111111111111114oLvT2', parse(17, bytes(20))) + self.assertEqual('31h1vYVSYuKP6AhS86fbRdMw9XHieotbST', parse(18, bytes(20))) + for nbytes in (0, 1, 19, 21, 32, 40): + self.assertIsNone(parse(17, bytes(nbytes))) + self.assertIsNone(parse(18, bytes(nbytes))) + # segwit v0: the witness program must be 20 or 32 bytes + self.assertEqual('bc1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq9e75rs', parse(0, bytes(20))) + self.assertEqual('bc1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqthqst8', parse(0, bytes(32))) + for nbytes in (0, 1, 19, 21, 40, 41): + self.assertIsNone(parse(0, bytes(nbytes))) + # segwit v1-v16: the witness program must be 2-40 bytes + for wver in (1, 16): + self.assertIsNotNone(parse(wver, bytes(2))) + self.assertIsNotNone(parse(wver, bytes(40))) + for nbytes in (0, 1, 41): + self.assertIsNone(parse(wver, bytes(nbytes))) + # unknown witness versions + self.assertIsNone(parse(19, bytes(20))) + self.assertIsNone(parse(31, bytes(20))) + # truncated/malformed frames + self.assertIsNone(parse_fallback_addr([], net)) + self.assertIsNone(parse_fallback_addr(bytearray(), net)) + self.assertIsNone(parse_fallback_addr([16, 1], net)) # non-zero padding bits + self.assertIsNone(parse_fallback_addr([17, 1], net)) + + def test_malformed_fallback_addr_is_skipped(self): + # BOLT #11: "A reader MUST skip over [...] an `f` field with unknown `version`". + # A malformed 'f' field must not abort decoding of the whole invoice. + for tagdata5 in ([], # empty payload + [16, 1], # non-zero padding bits + [17] + [0] * 4, # p2pkh with a too-short hash160 + [0] + [0] * 4, # p2wpkh with a too-short witness program + [19, 0, 0]): # unknown witness version + lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tag('f', tagdata5)) + self.assertIsNone(lnaddr.get_tag('f')) + self.assertEqual('', lnaddr.get_fallback_address()) + self.assertEqual(['f'], [tag for tag, _ in lnaddr.unknown_tags]) + # sanity: a well-formed 'f' field is still parsed + lnaddr = decode_bolt11_invoice( + self._encode_invoice_with_raw_tag('f', [17] + list(convertbits(bytes(20), 8, 5)))) + self.assertEqual('1111111111111111111114oLvT2', lnaddr.get_fallback_address()) + self.assertEqual([], lnaddr.unknown_tags) + + def test_tag_padding_errors(self): + # A tag whose 5->8 bit conversion has non-zero padding bits (or a length that cannot + # be converted at all) is malformed: it must be rejected, not crash the parser. + for tag, tagdata5 in (('d', [1]), # 5 bits left over: no valid conversion + ('d', [0, 1]), # non-zero padding bits + ('h', [0] * 51 + [1]), # data_length 52, non-zero padding bits + ('p', [0] * 51 + [1]), + ('s', [0] * 51 + [1]), + ('n', [0] * 52 + [1]), # data_length 53, non-zero padding bit + ('r', [1]), + ('r', [0, 1]), + ('t', [1]), + ('t', [0, 1])): + with self.subTest(tag=tag, tagdata5=tagdata5): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tag(tag, tagdata5)) + + # control: the same lengths with zero padding bits decode fine + for tag, tagdata5 in (('d', [0, 0]), + ('h', [0] * 51 + [16]), + ('p', [0] * 51 + [16]), + ('s', [0] * 51 + [16]), + ('n', list(convertbits(PUBKEY, 8, 5))), + ('r', [0] * 8), + ('t', [0] * 8)): + with self.subTest(tag=tag): + decode_bolt11_invoice(self._encode_invoice_with_raw_tag(tag, tagdata5)) + + # 'h', 'p', 's' and 'n' have a fixed data_length: a wrong length rejects the invoice + for tag, data_length in (('h', 52), ('p', 52), ('s', 52), ('n', 53)): + for wrong_length in (data_length - 1, data_length + 1): + with self.subTest(tag=tag, data_length=wrong_length): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice( + self._encode_invoice_with_raw_tag(tag, [0] * wrong_length)) + + # 'r' and 't': an empty payload converts to b'' instead of failing, so it is skipped + for tag in ('r', 't'): + with self.subTest(tag=tag, tagdata5=[]): + lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tag(tag, [])) + self.assertIsNone(lnaddr.get_tag(tag)) + self.assertEqual([], lnaddr.unknown_tags) + + # control: a well-formed hop is parsed + r_hop = bytes(33) + bytes(8) + (1).to_bytes(4, 'big') + (2).to_bytes(4, 'big') + (3).to_bytes(2, 'big') + t_hop = bytes(33) + (1).to_bytes(4, 'big') + (2).to_bytes(4, 'big') + (3).to_bytes(2, 'big') + for tag, hop in (('r', r_hop), ('t', t_hop)): + with self.subTest(tag=tag): + invoice = self._encode_invoice_with_raw_tag(tag, list(convertbits(hop, 8, 5))) + self.assertEqual(1, len(decode_bolt11_invoice(invoice).get_routing_info(tag))) + + def test_invalid_signature(self): + # The trailing 65 bytes of an invoice are attacker-controlled: every way the ecc lib + # can reject them must surface as BOLT11DecodeException, not leak out of the parser. + r_ok = (1).to_bytes(32, 'big') + s_ok = (1).to_bytes(32, 'big') + + # the recovery id (the last byte) must be 0-3 + for recid in (4, 27, 255): + with self.subTest(recid=recid): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_sig(r_ok + s_ok + bytes([recid]))) + + # r and s must be below the curve order + for label, sig64 in (('r == n', ecc.CURVE_ORDER.to_bytes(32, 'big') + s_ok), + ('r == n+1', (ecc.CURVE_ORDER + 1).to_bytes(32, 'big') + s_ok), + ('r == 2**256-1', b'\xff' * 32 + s_ok), + ('s == n', r_ok + ecc.CURVE_ORDER.to_bytes(32, 'big')), + ('s == n+1', r_ok + (ecc.CURVE_ORDER + 1).to_bytes(32, 'big')), + ('s == 2**256-1', r_ok + b'\xff' * 32)): + with self.subTest(sig=label): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_sig(sig64 + b'\x00')) + + # in-range but unrecoverable signature + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_sig(r_ok + s_ok + b'\x03')) + + # an 'n' field that is not a valid curve point (this path uses ecdsa_verify, not recovery) + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tag('n', list(convertbits(bytes(33), 8, 5)))) + def test_min_final_cltv_expiry_decoding(self): lnaddr = decode_bolt11_invoice("lnsb500u1pdsgyf3pp5nmrqejdsdgs4n9ukgxcp2kcq265yhrxd4k5dyue58rxtp5y83s3qsp5qyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqsdqqcqzys9qypqsqp2h6a5xeytuc3fad2ed4gxvhd593lwjdna3dxsyeem0qkzjx6guk44jend0xq4zzvp6f3fy07wnmxezazzsxgmvqee8shxjuqu2eu0qpnvc95x", net=constants.BitcoinSimnet) From 80c7bebc076f2355525a99e8b2b50c6ac14ea04d Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 10:51:34 +0200 Subject: [PATCH 2/9] bolt11: convert Bolt11Addr.date to property, add sanity check. route data/amount constructor arguments through setters, so we don't skip validation --- electrum/bolt11.py | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/electrum/bolt11.py b/electrum/bolt11.py index e96e03d70c8c..96bdeda836c1 100644 --- a/electrum/bolt11.py +++ b/electrum/bolt11.py @@ -22,6 +22,8 @@ if TYPE_CHECKING: from .lnutil import LnFeatures +TIMESTAMP_SANE_MAX = 2**35 + class BOLT11InvoiceException(Exception): pass class BOLT11DecodeException(BOLT11InvoiceException): pass @@ -269,10 +271,10 @@ class BOLT11Addr: def __init__( self, *, paymenthash: bytes = None, - amount: Optional[Decimal] = None, + amount: Optional[int | Decimal] = None, net: Type[AbstractNet] = None, tags: Optional[List[Tuple[str, Any]]] = None, - date: Optional[int] = None, + date: Optional[int | float] = None, payment_secret: bytes = None ): self.date = int(time.time()) if not date else int(date) @@ -283,19 +285,21 @@ def __init__( self.signature = None self.pubkey = None self.net = constants.net if net is None else net # type: Type[AbstractNet] - self._amount = amount # type: Optional[Decimal] # in bitcoins + self.amount = amount # type: Optional[int | Decimal] # in bitcoins @property def amount(self) -> Optional[Decimal]: return self._amount @amount.setter - def amount(self, value): - if not (isinstance(value, Decimal) or value is None): - raise BOLT11InvoiceException(f"amount must be Decimal or None, not {value!r}") + def amount(self, value: Optional[int | Decimal]): + if not (isinstance(value, (int, Decimal)) or value is None): + raise BOLT11InvoiceException(f"amount must be Decimal, int or None, not {value!r}") if value is None: self._amount = None return + if isinstance(value, int): + value = Decimal(value) assert isinstance(value, Decimal) if value.is_nan() or not (0 <= value <= TOTAL_COIN_SUPPLY_LIMIT_IN_BTC): raise BOLT11InvoiceException(f"amount is out-of-bounds: {value!r} BTC") @@ -304,6 +308,21 @@ def amount(self, value): raise BOLT11InvoiceException(f"Cannot encode {value!r}: too many decimal places") self._amount = value + @property + def date(self) -> int: + return self._date + + @date.setter + def date(self, value: int | float): + if value is None or not isinstance(value, (int, float)): + raise BOLT11InvoiceException(f"date must be int or float, not {value!r}") + if isinstance(value, float): + # e.g. from time.time() + value = int(value) + if value > TIMESTAMP_SANE_MAX: + raise BOLT11InvoiceException(f"date must be sane, not above {TIMESTAMP_SANE_MAX!r}") + self._date = value + def get_amount_sat(self) -> Optional[Decimal]: # note that this has msat resolution potentially if self.amount is None: @@ -477,11 +496,15 @@ def _convertbits_tag( # A reader SHOULD indicate if amount is unspecified, otherwise it MUST # multiply `amount` by the `multiplier` value (if any) to derive the # amount required for payment. - if amountstr != '': - addr.amount = unshorten_amount(amountstr) - - addr.date = int_from_data5(data5_remaining[:7]) - data5_remaining = data5_remaining[7:] + try: + if amountstr != '': + addr.amount = unshorten_amount(amountstr) + + addr.date = int_from_data5(data5_remaining[:7]) + data5_remaining = data5_remaining[7:] + except BOLT11InvoiceException as e: + # raise as decode exception, as amount/date comes from the encoded invoice + raise BOLT11DecodeException(f"Failed to decode invoice: {e}") from e while data5_remaining: try: From f07c822b99abbf07a938cf16b68b8a5e1b118283 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Mon, 7 Sep 2026 13:41:20 +0200 Subject: [PATCH 3/9] bip21: catch urlparse exceptions and raise as InvalidBitcoinURI --- electrum/bip21.py | 10 +++++++--- tests/test_util.py | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/electrum/bip21.py b/electrum/bip21.py index 3db495e7b54b..ecc4039369b1 100644 --- a/electrum/bip21.py +++ b/electrum/bip21.py @@ -7,7 +7,7 @@ from . import bitcoin from .util import format_satoshis_plain from .bitcoin import COIN, TOTAL_COIN_SUPPLY_LIMIT_IN_BTC -from .bolt11 import decode_bolt11_invoice, BOLT11DecodeException +from .bolt11 import decode_bolt11_invoice, BOLT11InvoiceException # note: when checking against these, use .lower() to support case-insensitivity BITCOIN_BIP21_URI_SCHEME = 'bitcoin' @@ -35,7 +35,11 @@ def parse_bip21_URI(uri: str) -> dict: raise InvalidBitcoinURI("Not a bitcoin address") return {'address': uri} - u = urllib.parse.urlparse(uri) + try: + u = urllib.parse.urlparse(uri) + except ValueError as e: + raise InvalidBitcoinURI("failed to parse uri") from e + if u.scheme.lower() != BITCOIN_BIP21_URI_SCHEME: raise InvalidBitcoinURI("Not a bitcoin URI") address = u.path @@ -94,7 +98,7 @@ def parse_bip21_URI(uri: str) -> dict: if 'lightning' in out: try: lnaddr = decode_bolt11_invoice(out['lightning']) - except BOLT11DecodeException as e: + except BOLT11InvoiceException as e: raise InvalidBitcoinURI(f"Failed to decode 'lightning' field: {e!r}") from e amount_sat = out.get('amount') if amount_sat: diff --git a/tests/test_util.py b/tests/test_util.py index 6be797ff5b5d..41123698c2c0 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -148,6 +148,12 @@ def test_parse_URI_invalid_address(self): def test_parse_URI_invalid(self): self.assertRaises(InvalidBitcoinURI, parse_bip21_URI, 'notbitcoin:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma') + def test_parse_URI_malformed(self): + # urllib.parse.urlparse itself raises ValueError on some inputs (an unterminated IPv6 + # netloc, a bad port); that must come out as InvalidBitcoinURI like any other bad URI + self.assertRaises(InvalidBitcoinURI, parse_bip21_URI, 'bitcoin://[::1') + self.assertRaises(InvalidBitcoinURI, parse_bip21_URI, 'bitcoin://[abc]:notaport') + def test_parse_URI_parameter_pollution(self): self.assertRaises(InvalidBitcoinURI, parse_bip21_URI, 'bitcoin:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=0.0003&label=test&amount=30.0') From 6870823f38bed418474a0760a494d3ad84e0abe4 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 11:21:08 +0200 Subject: [PATCH 4/9] bolt11: remove old tests without payment_secret We implement exactly the DB upgrade the comment there describes, so the old tests can be removed. --- tests/test_bolt11.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/test_bolt11.py b/tests/test_bolt11.py index fcef42f8e1b1..6ff82687ab60 100644 --- a/tests/test_bolt11.py +++ b/tests/test_bolt11.py @@ -103,19 +103,6 @@ def test_roundtrip(self): (BOLT11Addr(date=timestamp, paymenthash=RHASH, payment_secret=PAYMENT_SECRET, amount=24, tags=[('h', longdescription), ('9', 10 + (1 << 9) + (1 << 14))]), "lnbc241ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygshp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qrss2f8kr98446xls02yndup2ynwjh46u8kdeuuncexx2hnets0j0064nyq25gkd6jnttldzt5qqtszum5dufvuvryxt204w2p24557udxgcp0nlwtw"), ] - # Some old tests follow that do not have payment_secret. Note that if the parser raised due to the lack of features/payment_secret, - # old wallets that have these invoices saved (as paid/expired), could not be opened (though we could do a db upgrade and delete them). - tests.extend([ - (BOLT11Addr(date=timestamp, paymenthash=RHASH, tags=[('d', '')]), - "lnbc1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqqd9n3kwjjwglnfne5p4rvkze998m3xcxrc8kunl5khkchlaqhwhlyztuuwkrglv47mqg96mcqjjx70hh9luaj4te0u4ww6aclxwve3fqpkmdxlj"), - (BOLT11Addr(date=timestamp, paymenthash=RHASH, amount=Decimal('0.001'), tags=[('d', '1 cup coffee'), ('x', 60)]), - "lnbc1m1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpu9rflz25dx0qw6kdg05u0c5hdc30yq6ga6ew4pz86n244va45nchns9zrs3wjxznsqnt37hz7pswvc56wvuhxcjyd6k3lqf4ujynyxuspmvr078"), - (BOLT11Addr(date=timestamp, paymenthash=RHASH, amount=Decimal('1'), tags=[('h', longdescription)]), - "lnbc11ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs2qjafckq94q3js6lvqz2kmenn9ysjejyj8fm4hlx0xtqhaxfzlxjappkgp0hmm40dnuan4v3jy83lqjup2n0fdzgysg049y9l9uc98qq07kfd3"), - (BOLT11Addr(date=timestamp, paymenthash=RHASH, net=constants.BitcoinTestnet, tags=[('f', 'mk2QpYatsKicvFVuTAQLBryyccRXMUaGHP'), ('h', longdescription)]), - "lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqfpp3x9et2e20v6pu37c5d9vax37wxq72un98hp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsr9zktgu78k8p9t8555ve37qwfvqn6ga37fnfwhgexmf20nzdpmuhwvuv7zra3xrh8y2ggxxuemqfsgka9x7uzsrcx8rfv85c8pmhq9gq4sampn"), - - ]) # Roundtrip for lnaddr1, invoice_str1 in tests: From bcaa716091d9debb61db90ea2f666cd049d41365 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 12:08:55 +0200 Subject: [PATCH 5/9] tests: fix tests with bolt11 invoices without payment secret replaces invoices in tests with invoices with payment secret replaces lnpeer transitive feature test from BASIC_MPP_OPT -> PAYMENT_SECRET_OPT to PAYMENT_SECRET_OPT -> VAR_ONION_OPT --- tests/qml/test_qml_types.py | 6 +++--- tests/test_lnpeer.py | 2 +- tests/test_payment_identifier.py | 11 ++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/qml/test_qml_types.py b/tests/qml/test_qml_types.py index ba14fe115f6e..1bfda24229d1 100644 --- a/tests/qml/test_qml_types.py +++ b/tests/qml/test_qml_types.py @@ -131,9 +131,9 @@ def test_qeamount_frominvoice(self): self.assertEqual(0, a.satsInt) self.assertEqual(0, a.msatsInt) - bolt11 = 'lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqj9n4evl6mr5aj9f58zp6fyjzup6ywn3x6sk8akg5v4tgn2q8g4fhx05wf6juaxu9760yp46454gpg5mtzgerlzezqcqvjnhjh8z3g2qqdhhwkj' + bolt11 = 'lnbc241ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygshp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qrss2f8kr98446xls02yndup2ynwjh46u8kdeuuncexx2hnets0j0064nyq25gkd6jnttldzt5qqtszum5dufvuvryxt204w2p24557udxgcp0nlwtw' invoice = Invoice.from_bech32(bolt11) a = QEAmount(from_invoice=invoice) - self.assertEqual(2_000_000, a.satsInt) - self.assertEqual(2_000_000_000, a.msatsInt) + self.assertEqual(2_400_000_000, a.satsInt) + self.assertEqual(2_400_000_000_000, a.msatsInt) self.assertFalse(a.isMax) diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py index a7e82b3f793a..93207832821b 100644 --- a/tests/test_lnpeer.py +++ b/tests/test_lnpeer.py @@ -719,7 +719,7 @@ async def try_paying_some_invoices(): with self.assertRaises(lnutil.UnknownEvenFeatureBits): result, log = await w1.pay_invoice(pay_req) # feature bits: not all transitive dependencies are set - invoice_features = LnFeatures((1 << 8) + (1 << 17)) + invoice_features = LnFeatures((1 << 14) + (1 << 17)) lnaddr, pay_req = self.prepare_invoice(w2, invoice_features=invoice_features) with self.assertRaises(lnutil.IncompatibleOrInsaneFeatures): result, log = await w1.pay_invoice(pay_req) diff --git a/tests/test_payment_identifier.py b/tests/test_payment_identifier.py index 7b9cd3d9a3ac..257e4daad6fa 100644 --- a/tests/test_payment_identifier.py +++ b/tests/test_payment_identifier.py @@ -69,7 +69,7 @@ def test_remove_uri_prefix(self): def test_bolt11(self): # no amount, no fallback address - bolt11 = 'lnbc1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygsdqq9qypqszpyrpe4tym8d3q87d43cgdhhlsrt78epu7u99mkzttmt2wtsx0304rrw50addkryfrd3vn3zy467vxwlmf4uz7yvntuwjr2hqjl9lw5cqwtp2dy' + bolt11 = 'lnbc1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygsr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qy9qsq9s2256hf3kmf539hhds9uh30expkc6ulzd29zatpz60g292lu85reau6e4zpdlchsg4eprgk96a82ejdlyhfqx684xdzzepklfx6r2cpga5nw2' for pi_str in [ f'{bolt11}', f' {bolt11}', @@ -95,7 +95,8 @@ def test_bolt11(self): self.assertFalse(pi.is_valid()) # amount, fallback address - bolt_11_w_fallback = 'lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqj9n4evl6mr5aj9f58zp6fyjzup6ywn3x6sk8akg5v4tgn2q8g4fhx05wf6juaxu9760yp46454gpg5mtzgerlzezqcqvjnhjh8z3g2qqdhhwkj' + bolt_11_w_fallback = 'lnbc241ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygsr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqs9qy9qsqfnk063vsrgjx7l6td6v42skuxql7epn5tmrl4qte2e78nqnsjlgjg3sgkxreqex5fw4c9chnvtc2hykqnyxr84zwfr8f3d9q3h0nfdgqenlzvj' + pi = PaymentIdentifier(None, bolt_11_w_fallback) self.assertTrue(pi.is_valid()) self.assertEqual(PaymentIdentifierType.BOLT11, pi.type) @@ -136,7 +137,7 @@ def test_bip21(self): self.assertEqual('unit_test', pi.bip21.get('message')) # amount, expired, message, lightning w matching amount - bip21 = 'bitcoin:1RustyRX2oai4EYYDpQGWvEL62BBGqN9T?amount=0.02&message=unit_test&time=1707382023&exp=3600&lightning=lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqj9n4evl6mr5aj9f58zp6fyjzup6ywn3x6sk8akg5v4tgn2q8g4fhx05wf6juaxu9760yp46454gpg5mtzgerlzezqcqvjnhjh8z3g2qqdhhwkj' + bip21 = 'bitcoin:1RustyRX2oai4EYYDpQGWvEL62BBGqN9T?amount=0.001&message=unit_test&time=1707382023&exp=3600&lightning=lnbc1m1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygsdq5xysxxatsyp3k7enxv4jsxqzpu9qy9qsqw8l2pulslacwjt86vle3sgfdmcct5v34gtcpfnujsf6ufqa7v7jzdpddnwgte82wkscdlwfwucrgn8z36rv9hzk5mukltteh0yqephqpk5vegu' pi = PaymentIdentifier(None, bip21) self.assertTrue(pi.is_available()) @@ -418,13 +419,13 @@ def test_email_and_domain(self): async def test_invoice_from_payment_identifier(self): # amount, expired, message, lightning w matching amount - bip21 = 'bitcoin:1RustyRX2oai4EYYDpQGWvEL62BBGqN9T?amount=0.02&message=unit_test&time=1707382023&exp=3600&lightning=lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqj9n4evl6mr5aj9f58zp6fyjzup6ywn3x6sk8akg5v4tgn2q8g4fhx05wf6juaxu9760yp46454gpg5mtzgerlzezqcqvjnhjh8z3g2qqdhhwkj' + bip21 = 'bitcoin:1RustyRX2oai4EYYDpQGWvEL62BBGqN9T?amount=0.001&message=unit_test&time=1707382023&exp=3600&lightning=lnbc1m1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygsdq5xysxxatsyp3k7enxv4jsxqzpu9qy9qsqw8l2pulslacwjt86vle3sgfdmcct5v34gtcpfnujsf6ufqa7v7jzdpddnwgte82wkscdlwfwucrgn8z36rv9hzk5mukltteh0yqephqpk5vegu' pi = PaymentIdentifier(None, bip21) invoice = invoice_from_payment_identifier(pi, None, None) self.assertTrue(isinstance(invoice, Invoice)) self.assertTrue(invoice.is_lightning()) - self.assertEqual(2_000_000_000, invoice.amount_msat) + self.assertEqual(100_000_000, invoice.amount_msat) text = 'bitter grass shiver impose acquire brush forget axis eager alone wine silver' d = restore_wallet_from_text__for_unittest(text, path=self.wallet2_path, config=self.config) From 3cc15f31e45a7fdd34f313ec95d417a57fba4cb9 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 13:37:04 +0200 Subject: [PATCH 6/9] bolt11: add tests for new validation rules --- tests/test_bolt11.py | 203 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 189 insertions(+), 14 deletions(-) diff --git a/tests/test_bolt11.py b/tests/test_bolt11.py index 6ff82687ab60..26ef35698d03 100644 --- a/tests/test_bolt11.py +++ b/tests/test_bolt11.py @@ -8,8 +8,8 @@ from electrum.bolt11 import (shorten_amount, unshorten_amount, BOLT11Addr, encode_bolt11_invoice, decode_bolt11_invoice, parse_fallback_addr, int_to_data5, tagged5, tagged8, - BOLT11DecodeException) -from electrum.segwit_addr import bech32_encode, bech32_decode, convertbits + BOLT11DecodeException, BOLT11InvoiceException, TIMESTAMP_SANE_MAX) +from electrum.segwit_addr import bech32_encode, bech32_decode, convertbits, CHARSET_INVERSE from electrum import segwit_addr from electrum.lnutil import UnknownEvenFeatureBits, LnFeatures, IncompatibleLightningFeatures from electrum import constants @@ -130,25 +130,33 @@ def test_n_decoding(self): self.assertEqual(lnaddr.pubkey.serialize(), PUBKEY) @staticmethod - def _encode_invoice_with_raw_tag(tag, tagdata5, *, net=None, date=1615922274, expiry=None) -> str: - """Builds a correctly signed invoice with one arbitrary (possibly malformed) tagged field.""" + def _encode_invoice_with_raw_tags(tags5, *, net=None, date=1615922274, amountstr='') -> str: + """Builds a correctly signed invoice containing exactly the given (tag, data5) fields.""" net = net or constants.BitcoinMainnet - hrp = 'ln' + net.BOLT11_HRP + hrp = 'ln' + net.BOLT11_HRP + amountstr data5 = list(int_to_data5(date, bit_len=35)) - if tag != 'p': - data5 += list(tagged8('p', RHASH)) - if tag != 's': - data5 += list(tagged8('s', PAYMENT_SECRET)) - if tag not in ('d', 'h'): # exactly one of 'd'/'h' must be present - data5 += list(tagged8('d', b'test')) - if expiry is not None: - data5 += list(tagged5('x', int_to_data5(expiry))) - data5 += list(tagged5(tag, list(tagdata5))) + for tag, tagdata5 in tags5: + data5 += list(tagged5(tag, list(tagdata5))) msg32 = sha256(hrp.encode('ascii') + bytes(convertbits(data5, 5, 8))).digest() sig = ecc.ECPrivkey(PRIVKEY).ecdsa_sign_recoverable(msg32, is_compressed=False) sig = bytes(sig[1:]) + bytes([sig[0] - 27]) return bech32_encode(segwit_addr.Encoding.BECH32, hrp, data5 + list(convertbits(sig, 8, 5, False))) + @staticmethod + def _encode_invoice_with_raw_tag(tag, tagdata5, *, net=None, date=1615922274, expiry=None) -> str: + """Builds a correctly signed invoice with one arbitrary (possibly malformed) tagged field.""" + tags5 = [] + if tag != 'p': + tags5.append(('p', convertbits(RHASH, 8, 5))) + if tag != 's': + tags5.append(('s', convertbits(PAYMENT_SECRET, 8, 5))) + if tag not in ('d', 'h'): # exactly one of 'd'/'h' must be present + tags5.append(('d', convertbits(b'test', 8, 5))) + if expiry is not None: + tags5.append(('x', int_to_data5(expiry))) + tags5.append((tag, tagdata5)) + return TestBolt11._encode_invoice_with_raw_tags(tags5, net=net, date=date) + @staticmethod def _encode_invoice_with_raw_sig(sig65, *, net=None) -> str: """Builds an invoice with an arbitrary (possibly invalid) 65-byte signature. @@ -292,6 +300,173 @@ def test_invalid_signature(self): with self.assertRaises(BOLT11DecodeException): decode_bolt11_invoice(self._encode_invoice_with_raw_tag('n', list(convertbits(bytes(33), 8, 5)))) + def test_mandatory_tags(self): + # BOLT #11: a reader MUST fail the payment if a 'p' or 's' field is missing, and MUST + # fail if neither a 'd' nor an 'h' field is present, or if both are present. + p5 = convertbits(RHASH, 8, 5) + s5 = convertbits(PAYMENT_SECRET, 8, 5) + d5 = convertbits(b'test', 8, 5) + h5 = convertbits(sha256(b'test').digest(), 8, 5) + # control: 'd' and 'h' are each enough on their own + lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tags([('p', p5), ('s', s5), ('d', d5)])) + self.assertEqual('test', lnaddr.get_description()) + self.assertEqual(RHASH, lnaddr.paymenthash) + self.assertEqual(PAYMENT_SECRET, lnaddr.payment_secret) + decode_bolt11_invoice(self._encode_invoice_with_raw_tags([('p', p5), ('s', s5), ('h', h5)])) + + for label, tags5 in (("no 'p' field", [('s', s5), ('d', d5)]), + ("no 's' field", [('p', p5), ('d', d5)]), + ("neither 'd' nor 'h'", [('p', p5), ('s', s5)]), + ("both 'd' and 'h'", [('p', p5), ('s', s5), ('d', d5), ('h', h5)]), + ("no tagged fields at all", [])): + with self.subTest(label): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tags(tags5)) + + def test_duplicate_tags(self): + p5 = convertbits(RHASH, 8, 5) + s5 = convertbits(PAYMENT_SECRET, 8, 5) + d5 = convertbits(b'test', 8, 5) + h5 = convertbits(sha256(b'test').digest(), 8, 5) + # a second copy of a field we only keep one value for is rejected + for tag, tags5 in (('p', [('p', p5), ('p', p5), ('s', s5), ('d', d5)]), + ('s', [('p', p5), ('s', s5), ('s', s5), ('d', d5)]), + ('d', [('p', p5), ('s', s5), ('d', d5), ('d', d5)]), + ('h', [('p', p5), ('s', s5), ('h', h5), ('h', h5)])): + with self.subTest(tag=tag): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tags(tags5)) + + # 'n' is the exception: BOLT #11 has writers put the most-preferred field first, so the + # first one is kept and the rest ignored. Note the invoice is signed by PUBKEY, so if + # the second 'n' were the one kept, signature validation against it would fail. + other_pubkey = ecc.ECPrivkey(bytes(31) + b'\x02').get_public_key_bytes(compressed=True) + self.assertNotEqual(PUBKEY, other_pubkey) + lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tags( + [('p', p5), ('s', s5), ('d', d5), + ('n', convertbits(PUBKEY, 8, 5)), ('n', convertbits(other_pubkey, 8, 5))])) + self.assertEqual(PUBKEY, lnaddr.pubkey.serialize()) + + def test_invalid_utf8_description(self): + # the 'd' field is UTF-8: an invalid encoding must be rejected, not crash the parser + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tag('d', convertbits(b'\xff\xfe', 8, 5))) + # control: non-ASCII UTF-8 decodes fine + description = 'ナンセンス 1杯' + lnaddr = decode_bolt11_invoice( + self._encode_invoice_with_raw_tag('d', convertbits(description.encode('utf-8'), 8, 5))) + self.assertEqual(description, lnaddr.get_description()) + + def test_corrupt_tag_data(self): + # A tagged field whose data_length runs past the end of the data part must be rejected. + # Note the signature is split off the end first, so what is left for the tag loop is + # attacker-controlled in length as well as content. + hrp = 'ln' + constants.BitcoinMainnet.BOLT11_HRP + body = list(int_to_data5(1615922274, bit_len=35)) + body += list(tagged8('p', RHASH)) + list(tagged8('s', PAYMENT_SECRET)) + list(tagged8('d', b'test')) + sig5 = [0] * (65 * 8 // 5) + + def encode(data5): + return bech32_encode(segwit_addr.Encoding.BECH32, hrp, data5) + + for label, trailer in (("data_length past end of data", [CHARSET_INVERSE['x'], 0, 20]), + ("1 stray data element", [CHARSET_INVERSE['x']]), + ("2 stray data elements", [CHARSET_INVERSE['x'], 0])): + with self.subTest(label): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(encode(body + trailer + sig5)) + # an invoice that is all tags and no signature: the last 65 bytes are taken to be the + # signature regardless, which leaves a truncated tag behind + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(encode(body)) + # ... and one shorter than a signature is rejected outright + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(encode(list(int_to_data5(1615922274, bit_len=35)))) + # control: the same body followed by a signature-sized (if bogus) trailer gets all the + # way past the tag loop, and fails on the signature instead + with self.assertRaisesRegex(BOLT11DecodeException, 'signature'): + decode_bolt11_invoice(encode(body + sig5)) + + def test_bech32_errors(self): + invoice = self._encode_invoice_with_raw_tag('x', int_to_data5(60)) + self.assertEqual(60, decode_bolt11_invoice(invoice).get_expiry()) # control + + for label, bad_invoice in (("corrupt checksum", invoice[:-1] + ('q' if invoice[-1] != 'q' else 'p')), + ("mixed case", invoice[:8].upper() + invoice[8:]), + ("empty string", ''), + ("no separator", 'lnbc'), + ("not an invoice", 'not an invoice')): + with self.subTest(label): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(bad_invoice) + + decoded = bech32_decode(invoice, ignore_long_length=True) + # bolt11 uses vanilla bech32; the same data encoded as bech32m must be rejected + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(bech32_encode(segwit_addr.Encoding.BECH32M, decoded.hrp, decoded.data)) + # hrp of another network, and one that is not a lightning invoice at all + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(invoice, net=constants.BitcoinTestnet) + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(bech32_encode(segwit_addr.Encoding.BECH32, 'bc', decoded.data)) + + def test_invalid_amount(self): + # the amount is part of the hrp; amounts the BOLT11Addr.amount setter rejects must + # surface as BOLT11DecodeException, not as a bare BOLT11InvoiceException + tags5 = [('p', convertbits(RHASH, 8, 5)), + ('s', convertbits(PAYMENT_SECRET, 8, 5)), + ('d', convertbits(b'test', 8, 5))] + self.assertEqual( # control + Decimal('0.0025'), + decode_bolt11_invoice(self._encode_invoice_with_raw_tags(tags5, amountstr='2500u')).amount) + for amountstr in ('21000001', # more than the total coin supply + '1p', # sub-millisatoshi precision + '25y', # invalid multiplier + '-1', + 'nan', + '1e3'): + with self.subTest(amountstr=amountstr): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tags(tags5, amountstr=amountstr)) + + def test_amount_validation(self): + addr = BOLT11Addr(paymenthash=RHASH, payment_secret=PAYMENT_SECRET, tags=[('d', '')]) + for label, value in (("str", '1'), + ("float", 1.5), + ("bytes", b'1'), + ("NaN", Decimal('nan')), + ("negative", Decimal(-1)), + ("more than the coin supply", Decimal(21_000_001)), + ("sub-millisatoshi", Decimal('0.0000000000001'))): + with self.subTest(label): + with self.assertRaises(BOLT11InvoiceException): + addr.amount = value + addr.amount = 1 # an int is accepted and converted + self.assertEqual(Decimal(1), addr.amount) + addr.amount = Decimal('0.00000000001') # 1 msat, the smallest encodable amount + self.assertEqual(Decimal('0.00000000001'), addr.amount) + addr.amount = None + self.assertIsNone(addr.amount) + + def test_date_validation(self): + addr = BOLT11Addr(paymenthash=RHASH, payment_secret=PAYMENT_SECRET, tags=[('d', '')]) + for label, value in (("str", '123'), + ("bytes", b'123'), + ("None", None), + ("above TIMESTAMP_SANE_MAX", TIMESTAMP_SANE_MAX + 1)): + with self.subTest(label): + with self.assertRaises(BOLT11InvoiceException): + addr.date = value + with self.assertRaises(BOLT11InvoiceException): + BOLT11Addr(date=TIMESTAMP_SANE_MAX + 1) + # a float (e.g. straight from time.time()) is truncated to an int + addr.date = 1615922274.9 + self.assertEqual(1615922274, addr.date) + # the largest timestamp the 35-bit bolt11 field can hold is still accepted + addr.date = 2 ** 35 - 1 + self.assertEqual(2 ** 35 - 1, addr.date) + self.assertLessEqual(2 ** 35 - 1, TIMESTAMP_SANE_MAX) + def test_min_final_cltv_expiry_decoding(self): lnaddr = decode_bolt11_invoice("lnsb500u1pdsgyf3pp5nmrqejdsdgs4n9ukgxcp2kcq265yhrxd4k5dyue58rxtp5y83s3qsp5qyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqsdqqcqzys9qypqsqp2h6a5xeytuc3fad2ed4gxvhd593lwjdna3dxsyeem0qkzjx6guk44jend0xq4zzvp6f3fy07wnmxezazzsxgmvqee8shxjuqu2eu0qpnvc95x", net=constants.BitcoinSimnet) From 1f78bb606ef7f9306815eb5ed5c8e3009f5a35d6 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 10:20:36 +0200 Subject: [PATCH 7/9] wallet_db: add DB conversion step, remove invoices not passing strict parsing. --- electrum/wallet_db.py | 43 +++++++++++++++-- tests/test_invoices.py | 31 ++++++++++++ tests/test_storage_upgrade.py | 90 +++++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 5 deletions(-) diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py index 737df1846187..7aa8e73c7404 100644 --- a/electrum/wallet_db.py +++ b/electrum/wallet_db.py @@ -35,6 +35,7 @@ from . import bitcoin from . import constants +from .bolt11 import BOLT11InvoiceException from .util import profiler, WalletFileException, multisig_type, TxMinedInfo, MyEncoder, bfh from .keystore import bip44_derivation from .transaction import (Transaction, TxOutpoint, tx_from_any, PartialTransaction, PartialTxOutput, BadHeaderMagic, @@ -72,7 +73,7 @@ def __init__(self, wallet_db: 'WalletDB'): # seed_version is now used for the version of the wallet file OLD_SEED_VERSION = 4 # electrum versions < 2.0 NEW_SEED_VERSION = 11 # electrum versions >= 2.0 -FINAL_SEED_VERSION = 72 # electrum >= 2.7 will set this to prevent +FINAL_SEED_VERSION = 73 # electrum >= 2.7 will set this to prevent # old versions from overwriting new format @@ -262,6 +263,7 @@ def upgrade(self): self._convert_version_70() self._convert_version_71() self._convert_version_72() + self._convert_version_73() self.put('seed_version', FINAL_SEED_VERSION) # just to be sure def _convert_wallet_type(self): @@ -930,13 +932,18 @@ def _convert_version_45(self): # the new key for all requests is a wallet address, not done here for name in ['invoices', 'payment_requests']: invoices = self.data.get(name, {}) - for key, item in invoices.items(): + for key, item in list(invoices.items()): is_lightning = item['type'] == 2 lightning_invoice = item['invoice'] if is_lightning else None outputs = item['outputs'] if not is_lightning else None bip70 = item['bip70'] if not is_lightning else None if is_lightning: - lnaddr = decode_bolt11_invoice(item['invoice']) + try: + lnaddr = decode_bolt11_invoice(item['invoice']) + except BOLT11InvoiceException as e: + self.logger.warning(f"removing {name} item {key} that fails bolt11 decode: {e}") + del invoices[key] + continue amount_msat = lnaddr.get_amount_msat() timestamp = lnaddr.date exp_delay = lnaddr.get_expiry() @@ -997,7 +1004,12 @@ def _convert_version_47(self): for key, item in list(requests.items()): lnaddr = item.get('lightning_invoice') if lnaddr: - lnaddr = decode_bolt11_invoice(lnaddr) + try: + lnaddr = decode_bolt11_invoice(lnaddr) + except BOLT11InvoiceException as e: + self.logger.warning(f"removing request {key} that fails bolt11 decode: {e}") + del requests[key] + continue rhash = lnaddr.paymenthash.hex() if key != rhash: requests[rhash] = item @@ -1047,7 +1059,12 @@ def _convert_version_51(self): if lightning_invoice is None: payment_hash = None else: - lnaddr = decode_bolt11_invoice(lightning_invoice) + try: + lnaddr = decode_bolt11_invoice(lightning_invoice) + except BOLT11InvoiceException as e: + self.logger.warning(f"removing request {key} that fails bolt11 decode: {e}") + del requests[key] + continue payment_hash = lnaddr.paymenthash.hex() item['payment_hash'] = payment_hash self.data['seed_version'] = 51 @@ -1480,6 +1497,22 @@ def _serialize_imported_channel_backup(cb: dict) -> str: channel_backups[channel_id] = _serialize_imported_channel_backup(storage) self.data['seed_version'] = 72 + def _convert_version_73(self): + from .bolt11 import decode_bolt11_invoice + if not self._is_upgrade_method_needed(72, 72): + return + # remove invoices not passing stricter bolt11 invoice parsing (https://github.com/spesmilo/electrum/pull/10940) + invoices = self.data.get('invoices', {}) + for key, item in list(invoices.items()): + lnaddr = item.get('lightning_invoice') + if lnaddr: + try: + decode_bolt11_invoice(lnaddr) + except BOLT11InvoiceException as e: + self.logger.warning(f"removing invoice {key} that fails bolt11 decode: {e}") + del invoices[key] + self.data['seed_version'] = 73 + def _convert_imported(self): if not self._is_upgrade_method_needed(0, 13): return diff --git a/tests/test_invoices.py b/tests/test_invoices.py index f188559c489b..7b889ba6dbf7 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -4,6 +4,7 @@ from electrum import util from electrum.simple_config import SimpleConfig from electrum.wallet import Standard_Wallet, Abstract_Wallet +from electrum.bolt11 import BOLT11DecodeException from electrum.invoices import PR_UNPAID, PR_PAID, PR_UNCONFIRMED, PR_BROADCASTING, BaseInvoice, Invoice, LN_EXPIRY_NEVER from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED from electrum.transaction import Transaction, PartialTxOutput @@ -256,6 +257,36 @@ async def test_arg_validation(self): with self.assertRaises(TypeError): invoice.exp = "asd" + async def test_malformed_route_tag_is_rejected(self): + # A bolt11 invoice with a malformed 'r'/'t' tag used to decode fine (the tag was silently + # skipped). It is now rejected, both when it arrives from outside and when it comes off + # disk: the attrs validator decodes strictly. What keeps that from making an old wallet + # file unloadable is db conversion 73, which purges such invoices; see + # TestStorageUpgrade.test_upgrade_removes_invoice_with_malformed_route_tag. + # Both strings below are correctly signed testnet invoices whose 'r'/'t' payload has + # non-zero padding bits; see TestBolt11._encode_invoice_with_raw_tag. + for tag, invoice_str in ( + ('r', 'lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq8w3jhxaqrqzq' + 'pxhlj48td8uen6qqvke0kwsx0uf3g9pqfg3sdetumr2lla597ahcjcqcn5v7yycysc39ua9r2l8qx527' + 'uthxfdgmhp47exeh98pv7facqmjed87'), + ('t', 'lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq8w3jhxaqtqzq' + 'pg3tvdu05w4rd9ccwjq80f5ujz89c5ltq5fhp8dqxg7aan38gs24z0pgx8xj4vvzt2su5fqpr35tz692' + 'czrwt6e56twh3v8l0t8hfkxsq5xtyfu'), + ): + with self.subTest(tag=tag): + with self.assertRaises(BOLT11DecodeException): + Invoice( + amount_msat=None, + message="mymsg", + time=1615922274, + exp=LN_EXPIRY_NEVER, + outputs=None, + height=0, + lightning_invoice=invoice_str, + ) + with self.assertRaises(InvoiceError): + Invoice.from_bech32(invoice_str) + class TestOutgoingInvoicesPaidCache(ElectrumTestCase): """test caching of paid-status for outgoing invoices""" diff --git a/tests/test_storage_upgrade.py b/tests/test_storage_upgrade.py index ae467b5137c4..7e912412135c 100644 --- a/tests/test_storage_upgrade.py +++ b/tests/test_storage_upgrade.py @@ -8,6 +8,7 @@ import electrum from electrum.wallet_db import WalletDBUpgrader, WalletDB, WalletRequiresUpgrade, WalletRequiresSplit +from electrum.bolt11 import BOLT11DecodeException from electrum.wallet import Wallet from electrum import constants from electrum import util @@ -340,6 +341,95 @@ async def test_upgrade_from_client_4_8_1_9dk_with_ln_chan_backups(self): assert db.get("imported_channel_backups").get("ddb06b023f24a587d96a9f113c02d266549d010a57d7b151c1f5332a9bbaafd5") \ == "0200017e634853dc47f0bc2f2e0d1054b302fcb414371ddbd889f29ba8aa4e8b62c7725d472c7b642b14176f275d6dca60c8d1ec5cfbf935169f1fe873e6bd0ad155da038863cf8ab91046230f561cd5b386cbff8309fa02e3f0c3ed161a3aeb64a643b9d5afba9b2a33f5c151b1d7570a019d5466d2023c119f6ad987a5243f026bb0dd00003e74623171357a64726430703772366d68353961726e636179763030326e727530347030706636653264756b6479326833707672726e67747336687473616a02f2fa10e1317153b9cca5c0af211bcdd48aac4cf67a6f4d1cb7de71857261a1190303a53b5175b7ad2de558fc1f140d129fc5dd0949f1fbfac28ce2c33b236fbc6ef00390000e3230332e3133322e39342e313936072602a1ceaaae7b1da9d2e679977615988c62903e93a2e5d972aff6f0441face4be10c87b61e091f3f786ca44dc25283557214686d5ddb138eeb9df484145b991356b" + @as_testnet + async def test_upgrade_removes_invoice_with_malformed_route_tag(self): + # Db conversion 72->73 drops stored invoices that fail bolt11 decoding. + # Older versions decoded a malformed 'r'/'t' tag by silently skipping it, so such an + # invoice can be sitting in a wallet file; without this conversion it would now abort + # the load in Invoice._validate_invoice_str, leaving the file unopenable. + # The older conversions that decode invoices themselves (45, 47, 51) drop such items + # the same way, so a file from before those versions upgrades too. + # The malformed invoices below are correctly signed, but their 'r'/'t' payload has + # non-zero padding bits; see TestBolt11._encode_invoice_with_raw_tag. + bad_r = ('lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq8w3jhxaqrqzq' + 'pxhlj48td8uen6qqvke0kwsx0uf3g9pqfg3sdetumr2lla597ahcjcqcn5v7yycysc39ua9r2l8qx527' + 'uthxfdgmhp47exeh98pv7facqmjed87') + bad_t = ('lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq8w3jhxaqtqzq' + 'pg3tvdu05w4rd9ccwjq80f5ujz89c5ltq5fhp8dqxg7aan38gs24z0pgx8xj4vvzt2su5fqpr35tz692' + 'czrwt6e56twh3v8l0t8hfkxsq5xtyfu') + good = ('lntb15u1p0m6lzupp5zqjthgvaad9mewmdjuehwddyze9d8zyxcc43zhaddeegt37sndgsdq4xysyymr0vd' + '4kzcmrd9hx7cqp7xqrrss9qy9qsqsp5vlhcs24hwm747w8f3uau2tlrdkvjaglffnsstwyamj84cxuhrn2' + 's8tut3jqumepu42azyyjpgqa4w9w03204zp9h4clk499y2umstl6s29hqyj8vv4as6zt5567ux7l3f66m8' + 'pjhk65zjaq2esezk7ll2kcpljewkg') + + def invoice_json(lightning_invoice): + return {'amount_msat': None, 'message': 'mymsg', 'time': 1615922274, 'exp': 0, + 'outputs': None, 'height': 0, 'bip70': None, + 'lightning_invoice': lightning_invoice} + + data = { + 'seed_version': 72, + 'wallet_type': 'imported', + 'addresses': {'tb1qmjzmg8nd4z56ar4fpngzsr6euktrhnjg9td385': {}}, + 'invoices': {'bad_r': invoice_json(bad_r), + 'bad_t': invoice_json(bad_t), + 'good': invoice_json(good)}, + } + db = self._load_db_from_json_string(wallet_json=json.dumps(data), upgrade=True) + self.assertEqual(73, db.get('seed_version')) + self.assertEqual(['good'], list(db.get_dict('invoices').keys())) + + # sanity: without the conversion (i.e. already at seed_version 73) the same file + # would not load at all + data['seed_version'] = 73 + with self.assertRaises(BOLT11DecodeException): + self._load_db_from_json_string(wallet_json=json.dumps(data), upgrade=True) + + # a pre-45 file: conversion 45 decodes the invoices itself and drops the bad ones + data['seed_version'] = 44 + data['invoices'] = {key: {'type': 2, 'invoice': invoice_str} + for key, invoice_str in (('bad_r', bad_r), ('bad_t', bad_t), ('good', good))} + db = self._load_db_from_json_string(wallet_json=json.dumps(data), upgrade=True) + self.assertEqual(73, db.get('seed_version')) + self.assertEqual(['good'], list(db.get_dict('invoices').keys())) + + @as_testnet + async def test_upgrade_removes_request_with_malformed_route_tag(self): + # Same as the above, for the receive side: conversions 45, 47 and 51 each decode the + # bolt11 str of stored payment_requests, and drop the ones that no longer decode. + # (73 does not have to: a modern Request only stores the payment_hash.) + bad_r = ('lntb1ps9zprzpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq8w3jhxaqrqzq' + 'pxhlj48td8uen6qqvke0kwsx0uf3g9pqfg3sdetumr2lla597ahcjcqcn5v7yycysc39ua9r2l8qx527' + 'uthxfdgmhp47exeh98pv7facqmjed87') + good = ('lntb15u1p0m6lzupp5zqjthgvaad9mewmdjuehwddyze9d8zyxcc43zhaddeegt37sndgsdq4xysyymr0vd' + '4kzcmrd9hx7cqp7xqrrss9qy9qsqsp5vlhcs24hwm747w8f3uau2tlrdkvjaglffnsstwyamj84cxuhrn2' + 's8tut3jqumepu42azyyjpgqa4w9w03204zp9h4clk499y2umstl6s29hqyj8vv4as6zt5567ux7l3f66m8' + 'pjhk65zjaq2esezk7ll2kcpljewkg') + good_rhash = '1024bba19deb4bbcbb6d97337735a4164ad38886c62b115fad6e7285c7d09b51' + + def request_json(seed_version, lightning_invoice): + """A payment_requests item holding a bolt11 str, in that seed_version's shape.""" + if seed_version < 45: + return {'type': 2, 'invoice': lightning_invoice} + # note: amount_msat must match the invoice and be an int, else conversion 54 drops it + return {'amount_msat': 1_500_000, 'message': 'mymsg', 'time': 1615922274, 'exp': 0, + 'outputs': None, 'height': 0, 'bip70': None, + 'lightning_invoice': lightning_invoice} + + # 44 -> conversion 45 drops it, 46 -> conversion 47 does, 50 -> conversion 51 does + for seed_version in (44, 46, 50): + with self.subTest(seed_version=seed_version): + data = { + 'seed_version': seed_version, + 'wallet_type': 'imported', + 'addresses': {'tb1qmjzmg8nd4z56ar4fpngzsr6euktrhnjg9td385': {}}, + 'payment_requests': {'bad_r': request_json(seed_version, bad_r), + good_rhash: request_json(seed_version, good)}, + } + db = self._load_db_from_json_string(wallet_json=json.dumps(data), upgrade=True) + self.assertEqual(73, db.get('seed_version')) + self.assertEqual([good_rhash], list(db.get_dict('payment_requests').keys())) + ########## From 7d73d4aaba96f4d218edb5504be49ea3d5047816 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Fri, 11 Sep 2026 14:10:12 +0200 Subject: [PATCH 8/9] bolt11: validate x/c/9 tags requirement for 'minimal' data --- electrum/bolt11.py | 12 ++++++++++++ tests/test_bolt11.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/electrum/bolt11.py b/electrum/bolt11.py index 96bdeda836c1..b274b6efc9cd 100644 --- a/electrum/bolt11.py +++ b/electrum/bolt11.py @@ -459,6 +459,11 @@ def _convertbits_tag( raise BOLT11DecodeException(f"Failed to decode tag '{tag}'") return intseq + def _check_minimal_data5(tag: str, data5: Sequence[int]) -> None: + """ a field is 'minimal' if len>0 and the first int is non-zero""" + if len(data5) > 0 and data5[0] == 0: + raise BOLT11DecodeException(f"Non-minimal data_length for tag '{tag}'") + if net is None: net = constants.net decoded_bech32 = bech32_decode(invoice, ignore_long_length=True) @@ -589,6 +594,8 @@ def _convertbits_tag( ('h', bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(52, 52)))) ) elif tag == 'x': + # MUST use the minimum data_length possible + _check_minimal_data5(tag, tagdata) addr.tags.append(('x', int_from_data5(tagdata))) elif tag == 'p': # MUST include exactly one 'p' field @@ -609,8 +616,13 @@ def _convertbits_tag( pubkeybytes = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(53, 53))) addr.pubkey = pubkeybytes elif tag == 'c': + # MUST use the minimum data_length possible + _check_minimal_data5(tag, tagdata) addr.tags.append(('c', int_from_data5(tagdata))) elif tag == '9': + # MUST use the minimum data_length possible to encode the non-zero bits, + # with no 0 field-elements at the start + _check_minimal_data5(tag, tagdata) features = int_from_data5(tagdata) addr.tags.append(('9', features)) # note: The features are not validated here in the parser, diff --git a/tests/test_bolt11.py b/tests/test_bolt11.py index 26ef35698d03..9cf475715ba2 100644 --- a/tests/test_bolt11.py +++ b/tests/test_bolt11.py @@ -357,6 +357,44 @@ def test_invalid_utf8_description(self): self._encode_invoice_with_raw_tag('d', convertbits(description.encode('utf-8'), 8, 5))) self.assertEqual(description, lnaddr.get_description()) + def test_non_minimal_data_length(self): + # BOLT #11: a 'c', 'x' or '9' field "MUST use the minimum `data_length` possible, i.e. + # no leading 0 field-elements"; a reader SHOULD treat a non-minimal one as invalid. + for tag in ('x', 'c', '9'): + for tagdata5 in ([0], # zero, which is minimally encoded as an empty field + [0, 1, 28], # 60, left-padded with one 0 element + [0] * 50 + [1]): # 1, left-padded all the way + with self.subTest(tag=tag, tagdata5=tagdata5): + with self.assertRaises(BOLT11DecodeException): + decode_bolt11_invoice(self._encode_invoice_with_raw_tag(tag, tagdata5)) + # control: minimally encoded values, zero (i.e. an empty field) included + for tagdata5, value in (([], 0), ([31], 31), ([1, 28], 60)): + with self.subTest(tag=tag, tagdata5=tagdata5): + lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tag(tag, tagdata5)) + self.assertEqual(value, lnaddr.get_tag(tag)) + + # the accessors see the decoded values + self.assertEqual(60, decode_bolt11_invoice( + self._encode_invoice_with_raw_tag('x', [1, 28])).get_expiry()) + self.assertEqual(31, decode_bolt11_invoice( + self._encode_invoice_with_raw_tag('c', [31])).get_min_final_cltv_delta()) + + # ... and whatever our own encoder emits is minimal, so it still roundtrips + for tag in ('x', 'c', '9'): + for value in (1, 31, 32, 60, 3600, 2 ** 40): + with self.subTest(tag=tag, value=value): + addr = BOLT11Addr(date=1615922274, paymenthash=RHASH, payment_secret=PAYMENT_SECRET, + tags=[('d', ''), (tag, value)]) + lnaddr = decode_bolt11_invoice(encode_bolt11_invoice(addr, PRIVKEY)) + self.assertEqual(value, lnaddr.get_tag(tag)) + # zero: 'x' and 'c' are written as an empty (still minimal) field, '9' is omitted entirely + for tag, expected in (('x', 0), ('c', 0), ('9', None)): + with self.subTest(tag=tag, value=0): + addr = BOLT11Addr(date=1615922274, paymenthash=RHASH, payment_secret=PAYMENT_SECRET, + tags=[('d', ''), (tag, 0)]) + lnaddr = decode_bolt11_invoice(encode_bolt11_invoice(addr, PRIVKEY)) + self.assertEqual(expected, lnaddr.get_tag(tag)) + def test_corrupt_tag_data(self): # A tagged field whose data_length runs past the end of the data part must be rejected. # Note the signature is split off the end first, so what is left for the tag loop is From 3026fbdb048958bac751f27a89b9c026b5a92bad Mon Sep 17 00:00:00 2001 From: f321x Date: Mon, 14 Sep 2026 15:47:04 +0200 Subject: [PATCH 9/9] bolt11: followup #10940 Address review comments of #10940 --- electrum/bolt11.py | 12 +++++------- tests/test_bolt11.py | 25 +++++++++---------------- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/electrum/bolt11.py b/electrum/bolt11.py index b274b6efc9cd..02cb9291a36f 100644 --- a/electrum/bolt11.py +++ b/electrum/bolt11.py @@ -22,7 +22,7 @@ if TYPE_CHECKING: from .lnutil import LnFeatures -TIMESTAMP_SANE_MAX = 2**35 +TIMESTAMP_SANE_MAX = 2**35 - 1 class BOLT11InvoiceException(Exception): pass @@ -319,8 +319,8 @@ def date(self, value: int | float): if isinstance(value, float): # e.g. from time.time() value = int(value) - if value > TIMESTAMP_SANE_MAX: - raise BOLT11InvoiceException(f"date must be sane, not above {TIMESTAMP_SANE_MAX!r}") + if not 0 <= value <= TIMESTAMP_SANE_MAX: + raise BOLT11InvoiceException(f"date must be in [0; {TIMESTAMP_SANE_MAX!r}]: {value}") self._date = value def get_amount_sat(self) -> Optional[Decimal]: @@ -608,11 +608,9 @@ def _check_minimal_data5(tag: str, data5: Sequence[int]) -> None: raise BOLT11DecodeException("Unexpected 's' tag") addr.payment_secret = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(52, 52))) elif tag == 'n': - # if a writer offers more than one of any field type, it: - # MUST specify the most-preferred field first, followed by less-preferred fields, in order. - # as we store a single pubkey, we only store the first + # MAY include one n field if addr.pubkey is not None: - continue + raise BOLT11DecodeException("Unexpected 'n' tag") pubkeybytes = bytes(_convertbits_tag(tag, tagdata, 5, 8, False, length_range=(53, 53))) addr.pubkey = pubkeybytes elif tag == 'c': diff --git a/tests/test_bolt11.py b/tests/test_bolt11.py index 9cf475715ba2..ddb8f296385c 100644 --- a/tests/test_bolt11.py +++ b/tests/test_bolt11.py @@ -165,6 +165,7 @@ def _encode_invoice_with_raw_sig(sig65, *, net=None) -> str: hrp = 'ln' + net.BOLT11_HRP data5 = list(int_to_data5(1615922274, bit_len=35)) data5 += list(tagged8('p', RHASH)) + data5 += list(tagged8('s', PAYMENT_SECRET)) data5 += list(tagged8('d', b'test')) return bech32_encode(segwit_addr.Encoding.BECH32, hrp, data5 + list(convertbits(sig65, 8, 5, False))) @@ -278,7 +279,7 @@ def test_invalid_signature(self): # the recovery id (the last byte) must be 0-3 for recid in (4, 27, 255): with self.subTest(recid=recid): - with self.assertRaises(BOLT11DecodeException): + with self.assertRaisesRegex(BOLT11DecodeException, r"^Invalid signature:"): decode_bolt11_invoice(self._encode_invoice_with_raw_sig(r_ok + s_ok + bytes([recid]))) # r and s must be below the curve order @@ -289,15 +290,15 @@ def test_invalid_signature(self): ('s == n+1', r_ok + (ecc.CURVE_ORDER + 1).to_bytes(32, 'big')), ('s == 2**256-1', r_ok + b'\xff' * 32)): with self.subTest(sig=label): - with self.assertRaises(BOLT11DecodeException): + with self.assertRaisesRegex(BOLT11DecodeException, r"^Invalid signature:"): decode_bolt11_invoice(self._encode_invoice_with_raw_sig(sig64 + b'\x00')) # in-range but unrecoverable signature - with self.assertRaises(BOLT11DecodeException): + with self.assertRaisesRegex(BOLT11DecodeException, r"^Invalid signature:"): decode_bolt11_invoice(self._encode_invoice_with_raw_sig(r_ok + s_ok + b'\x03')) # an 'n' field that is not a valid curve point (this path uses ecdsa_verify, not recovery) - with self.assertRaises(BOLT11DecodeException): + with self.assertRaisesRegex(BOLT11DecodeException, r"^Invalid signature:"): decode_bolt11_invoice(self._encode_invoice_with_raw_tag('n', list(convertbits(bytes(33), 8, 5)))) def test_mandatory_tags(self): @@ -328,25 +329,17 @@ def test_duplicate_tags(self): s5 = convertbits(PAYMENT_SECRET, 8, 5) d5 = convertbits(b'test', 8, 5) h5 = convertbits(sha256(b'test').digest(), 8, 5) + n5 = convertbits(PUBKEY, 8, 5) # a second copy of a field we only keep one value for is rejected for tag, tags5 in (('p', [('p', p5), ('p', p5), ('s', s5), ('d', d5)]), ('s', [('p', p5), ('s', s5), ('s', s5), ('d', d5)]), ('d', [('p', p5), ('s', s5), ('d', d5), ('d', d5)]), - ('h', [('p', p5), ('s', s5), ('h', h5), ('h', h5)])): + ('h', [('p', p5), ('s', s5), ('h', h5), ('h', h5)]), + ('n', [('p', p5), ('s', s5), ('d', d5), ('n', n5), ('n', n5)])): with self.subTest(tag=tag): - with self.assertRaises(BOLT11DecodeException): + with self.assertRaisesRegex(BOLT11DecodeException, f"^Unexpected (multiple )?'{tag}' tags?$"): decode_bolt11_invoice(self._encode_invoice_with_raw_tags(tags5)) - # 'n' is the exception: BOLT #11 has writers put the most-preferred field first, so the - # first one is kept and the rest ignored. Note the invoice is signed by PUBKEY, so if - # the second 'n' were the one kept, signature validation against it would fail. - other_pubkey = ecc.ECPrivkey(bytes(31) + b'\x02').get_public_key_bytes(compressed=True) - self.assertNotEqual(PUBKEY, other_pubkey) - lnaddr = decode_bolt11_invoice(self._encode_invoice_with_raw_tags( - [('p', p5), ('s', s5), ('d', d5), - ('n', convertbits(PUBKEY, 8, 5)), ('n', convertbits(other_pubkey, 8, 5))])) - self.assertEqual(PUBKEY, lnaddr.pubkey.serialize()) - def test_invalid_utf8_description(self): # the 'd' field is UTF-8: an invalid encoding must be rejected, not crash the parser with self.assertRaises(BOLT11DecodeException):