electrum

commit ac223073ba7998901bd9a10d32594ebf0583021f
parent 0c3f36038599401c226addb05249c964247579da
Author: SomberNight <somber.night@protonmail.com>
Date:   Wed,  9 Dec 2020 09:38:40 +0100

keystore: handle unusual derivation paths in PSBT

If a tx contained a derivation path for a pubkey,
with a length=2 der suffix,
with the first element of the suffix not in (0, 1),
with a fingerprint that matches either our root or intermediate fp,
then processing that tx would raise and result in a crash reporter.

Traceback (most recent call last):
  File ".../electrum/electrum/gui/qt/main_window.py", line 2718, in do_process_from_text
    self.show_transaction(tx)
  File ".../electrum/electrum/gui/qt/main_window.py", line 1041, in show_transaction
    show_transaction(tx, parent=self, desc=tx_desc)
  File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 84, in show_transaction
    d = TxDialog(tx, parent=parent, desc=desc, prompt_if_unsaved=prompt_if_unsaved)
  File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 680, in __init__
    self.set_tx(tx)
  File ".../electrum/electrum/gui/qt/transaction_dialog.py", line 218, in set_tx
    tx.add_info_from_wallet(self.wallet)
  File ".../electrum/electrum/transaction.py", line 1944, in add_info_from_wallet
    wallet.add_input_info(txin, only_der_suffix=only_der_suffix)
  File ".../electrum/electrum/wallet.py", line 1573, in add_input_info
    is_mine = self._learn_derivation_path_for_address_from_txinout(txin, address)
  File ".../electrum/electrum/wallet.py", line 2609, in _learn_derivation_path_for_address_from_txinout
    pubkey, der_suffix = ks.find_my_pubkey_in_txinout(txinout, only_der_suffix=True)
  File ".../electrum/electrum/keystore.py", line 155, in find_my_pubkey_in_txinout
    path = self.get_pubkey_derivation(pubkey, txinout, only_der_suffix=only_der_suffix)
  File ".../electrum/electrum/keystore.py", line 391, in get_pubkey_derivation
    if not test_der_suffix_against_pubkey(der_suffix, pubkey):
  File ".../electrum/electrum/keystore.py", line 368, in test_der_suffix_against_pubkey
    if pubkey != self.derive_pubkey(*der_suffix):
  File ".../electrum/electrum/keystore.py", line 491, in derive_pubkey
    assert for_change in (0, 1)
AssertionError

Diffstat:
M
electrum/keystore.py
 | 
28
+++++++++++++++++++++-------
M
electrum/wallet.py
 | 
8
++++++--

2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/electrum/keystore.py b/electrum/keystore.py
@@ -52,6 +52,9 @@ if TYPE_CHECKING:
     from .wallet_db import WalletDB
 
 
+class CannotDerivePubkey(Exception): pass
+
+
 class KeyStore(Logger, ABC):
     type: str
 
@@ -356,16 +359,25 @@ class MasterPublicKeyMixin(ABC):
 
     @abstractmethod
     def derive_pubkey(self, for_change: int, n: int) -> bytes:
+        """Returns pubkey at given path.
+        May raise CannotDerivePubkey.
+        """
         pass
 
-    def get_pubkey_derivation(self, pubkey: bytes,
-                              txinout: Union['PartialTxInput', 'PartialTxOutput'],
-                              *, only_der_suffix=True) \
-            -> Union[Sequence[int], str, None]:
+    def get_pubkey_derivation(
+            self,
+            pubkey: bytes,
+            txinout: Union['PartialTxInput', 'PartialTxOutput'],
+            *,
+            only_der_suffix=True,
+    ) -> Union[Sequence[int], str, None]:
         def test_der_suffix_against_pubkey(der_suffix: Sequence[int], pubkey: bytes) -> bool:
             if len(der_suffix) != 2:
                 return False
-            if pubkey != self.derive_pubkey(*der_suffix):
+            try:
+                if pubkey != self.derive_pubkey(*der_suffix):
+                    return False
+            except CannotDerivePubkey:
                 return False
             return True
 
@@ -488,7 +500,8 @@ class Xpub(MasterPublicKeyMixin):
     @lru_cache(maxsize=None)
     def derive_pubkey(self, for_change: int, n: int) -> bytes:
         for_change = int(for_change)
-        assert for_change in (0, 1)
+        if for_change not in (0, 1):
+            raise CannotDerivePubkey("forbidden path")
         xpub = self.xpub_change if for_change else self.xpub_receive
         if xpub is None:
             rootnode = self.get_bip32_node_for_xpub()
@@ -658,7 +671,8 @@ class Old_KeyStore(MasterPublicKeyMixin, Deterministic_KeyStore):
     @lru_cache(maxsize=None)
     def derive_pubkey(self, for_change, n) -> bytes:
         for_change = int(for_change)
-        assert for_change in (0, 1)
+        if for_change not in (0, 1):
+            raise CannotDerivePubkey("forbidden path")
         return self.get_pubkey_from_mpk(self.mpk, for_change, n)
 
     def _get_private_key_from_stretched_exponent(self, for_change, n, secexp):
diff --git a/electrum/wallet.py b/electrum/wallet.py
@@ -61,7 +61,8 @@ from .bitcoin import COIN, TYPE_ADDRESS
 from .bitcoin import is_address, address_to_script, is_minikey, relayfee, dust_threshold
 from .crypto import sha256d
 from . import keystore
-from .keystore import load_keystore, Hardware_KeyStore, KeyStore, KeyStoreWithMPK, AddressIndexGeneric
+from .keystore import (load_keystore, Hardware_KeyStore, KeyStore, KeyStoreWithMPK,
+                       AddressIndexGeneric, CannotDerivePubkey)
 from .util import multisig_type
 from .storage import StorageEncryptionVersion, WalletStorage
 from .wallet_db import WalletDB
@@ -2611,7 +2612,10 @@ class Deterministic_Wallet(Abstract_Wallet):
                 # note: we already know the pubkey belongs to the keystore,
                 #       but the script template might be different
                 if len(der_suffix) != 2: continue
-                my_address = self.derive_address(*der_suffix)
+                try:
+                    my_address = self.derive_address(*der_suffix)
+                except CannotDerivePubkey:
+                    my_address = None
                 if my_address == address:
                     self._ephemeral_addr_to_addr_index[address] = list(der_suffix)
                     return True