Fix 10937

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:

electrum/bolt11.py

@@ -13,7 +13,7 @@
 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 (caller should skip the field)."""
+    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
@@ -404,17 +415,29 @@ def serialize(self):
         return self.pubkey.get_public_key_bytes(True)
 
 
-def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Addr:
+def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None, strict=True) -> BOLT11Addr:
     """Parses a string into a BOLT11Addr object.
-    Can raise BOLT11DecodeException or IncompatibleOrInsaneFeatures.
+    Can raise BOLT11DecodeException.
+
+    :param strict:
+        if True, fail decode on malformed 'r' and 't' tags. False restores old behavior.
+        False is used when migrating wallet_db: an invoice stored by an older version can be
+        malformed.
     """
+
+    def _convertbits_tag(tag, *args, **kwargs):
+        if (intseq := convertbits(*args, **kwargs)) 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,7 +474,10 @@ 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:
         #
@@ -472,6 +498,8 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add
             #    * `feerate` (32 bits, big-endian)
             #    * `cltv_expiry_delta` (16 bits, big-endian)
             tagdata = convertbits(tagdata, 5, 8, False)
+            if strict and tagdata is None:
+                raise BOLT11DecodeException(f"Failed to decode tag '{tag}'")
             if not tagdata:
                 continue
             route = []
@@ -492,6 +520,8 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add
                 addr.tags.append(('r',route))
         elif tag == 't':
             tagdata = convertbits(tagdata, 5, 8, False)
+            if strict and tagdata is None:
+                raise BOLT11DecodeException(f"Failed to decode tag '{tag}'")
             if not tagdata:
                 continue
             route = []
@@ -516,13 +546,15 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add
                 continue
 
         elif tag == 'd':
-            addr.tags.append(('d', bytes(convertbits(tagdata, 5, 8, False)).decode('utf-8')))
-
+            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))))
+            addr.tags.append(('h', bytes(_convertbits_tag(tag, tagdata, 5, 8, False))))
 
         elif tag == 'x':
             addr.tags.append(('x', int_from_data5(tagdata)))
@@ -531,19 +563,19 @@ def decode_bolt11_invoice(invoice: str, *, verbose=False, net=None) -> BOLT11Add
             if data_length != 52:
                 addr.unknown_tags.append((tag, tagdata))
                 continue
-            addr.paymenthash = bytes(convertbits(tagdata, 5, 8, False))
+            addr.paymenthash = bytes(_convertbits_tag(tag, tagdata, 5, 8, False))
 
         elif tag == 's':
             if data_length != 52:
                 addr.unknown_tags.append((tag, tagdata))
                 continue
-            addr.payment_secret = bytes(convertbits(tagdata, 5, 8, False))
+            addr.payment_secret = bytes(_convertbits_tag(tag, tagdata, 5, 8, False))
 
         elif tag == 'n':
             if data_length != 53:
                 addr.unknown_tags.append((tag, tagdata))
                 continue
-            pubkeybytes = bytes(convertbits(tagdata, 5, 8, False))
+            pubkeybytes = bytes(_convertbits_tag(tag, tagdata, 5, 8, False))
             addr.pubkey = pubkeybytes
 
         elif tag == 'c':
@@ -575,20 +607,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

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):
@@ -936,7 +938,7 @@ def _convert_version_45(self):
                 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'])
+                    lnaddr = decode_bolt11_invoice(item['invoice'], strict=False)
                     amount_msat = lnaddr.get_amount_msat()
                     timestamp = lnaddr.date
                     exp_delay = lnaddr.get_expiry()
@@ -997,7 +999,7 @@ def _convert_version_47(self):
         for key, item in list(requests.items()):
             lnaddr = item.get('lightning_invoice')
             if lnaddr:
-                lnaddr = decode_bolt11_invoice(lnaddr)
+                lnaddr = decode_bolt11_invoice(lnaddr, strict=False)
                 rhash = lnaddr.paymenthash.hex()
                 if key != rhash:
                     requests[rhash] = item
@@ -1047,7 +1049,7 @@ def _convert_version_51(self):
             if lightning_invoice is None:
                 payment_hash = None
             else:
-                lnaddr = decode_bolt11_invoice(lightning_invoice)
+                lnaddr = decode_bolt11_invoice(lightning_invoice, strict=False)
                 payment_hash = lnaddr.paymenthash.hex()
             item['payment_hash'] = payment_hash
         self.data['seed_version'] = 51
@@ -1480,6 +1482,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 strict bolt11 invoice check
+        invoices = self.data.get('invoices', {})
+        for key, item in list(invoices.items()):
+            lnaddr = item.get('lightning_invoice')
+            if lnaddr:
+                try:
+                    decode_bolt11_invoice(lnaddr, strict=True)
+                except BOLT11InvoiceException as e:
+                    self.logger.warning(f"removing invoice {key} that fails strict bolt11 check: {e}")
+                    del invoices[key]
+        self.data['seed_version'] = 73
+
     def _convert_imported(self):
         if not self._is_upgrade_method_needed(0, 13):
             return