wallet: stricter validation in export_private_key

fixes #5422

Author: SomberNight

electrum/bip32.py

@@ -200,6 +200,8 @@ def is_private(self) -> bool:
         return isinstance(self.eckey, ecc.ECPrivkey)
 
     def subkey_at_private_derivation(self, path: Union[str, Iterable[int]]) -> 'BIP32Node':
+        if path is None:
+            raise Exception("derivation path must not be None")
         if isinstance(path, str):
             path = convert_bip32_path_to_list_of_uint32(path)
         if not self.is_private():
@@ -224,6 +226,8 @@ def subkey_at_private_derivation(self, path: Union[str, Iterable[int]]) -> 'BIP3
                          child_number=child_number)
 
     def subkey_at_public_derivation(self, path: Union[str, Iterable[int]]) -> 'BIP32Node':
+        if path is None:
+            raise Exception("derivation path must not be None")
         if isinstance(path, str):
             path = convert_bip32_path_to_list_of_uint32(path)
         if not path:

electrum/tests/test_commands.py

@@ -75,6 +75,43 @@ def test_encrypt_decrypt(self, mock_write):
         ciphertext = cmds.encrypt(pubkey, cleartext)
         self.assertEqual(cleartext, cmds.decrypt(pubkey, ciphertext))
 
+    @mock.patch.object(storage.WalletStorage, '_write')
+    def test_export_private_key_imported(self, mock_write):
+        wallet = restore_wallet_from_text('p2wpkh:L4rYY5QpfN6wJEF4SEKDpcGhTPnCe9zcGs6hiSnhpprZqVywFifN p2wpkh:L4jkdiXszG26SUYvwwJhzGwg37H2nLhrbip7u6crmgNeJysv5FHL',
+                                          path='if_this_exists_mocking_failed_648151893')['wallet']
+        cmds = Commands(config=None, wallet=wallet, network=None)
+        # single address tests
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys("asdasd")  # invalid addr, though might raise "not in wallet"
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys("bc1qgfam82qk7uwh5j2xxmcd8cmklpe0zackyj6r23")  # not in wallet
+        self.assertEqual("p2wpkh:L4jkdiXszG26SUYvwwJhzGwg37H2nLhrbip7u6crmgNeJysv5FHL",
+                         cmds.getprivatekeys("bc1q2ccr34wzep58d4239tl3x3734ttle92a8srmuw"))
+        # list of addresses tests
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys(['bc1q2ccr34wzep58d4239tl3x3734ttle92a8srmuw', 'asd'])
+        self.assertEqual(['p2wpkh:L4jkdiXszG26SUYvwwJhzGwg37H2nLhrbip7u6crmgNeJysv5FHL', 'p2wpkh:L4rYY5QpfN6wJEF4SEKDpcGhTPnCe9zcGs6hiSnhpprZqVywFifN'],
+                         cmds.getprivatekeys(['bc1q2ccr34wzep58d4239tl3x3734ttle92a8srmuw', 'bc1q9pzjpjq4nqx5ycnywekcmycqz0wjp2nq604y2n']))
+
+    @mock.patch.object(storage.WalletStorage, '_write')
+    def test_export_private_key_deterministic(self, mock_write):
+        wallet = restore_wallet_from_text('bitter grass shiver impose acquire brush forget axis eager alone wine silver',
+                                          gap_limit=2,
+                                          path='if_this_exists_mocking_failed_648151893')['wallet']
+        cmds = Commands(config=None, wallet=wallet, network=None)
+        # single address tests
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys("asdasd")  # invalid addr, though might raise "not in wallet"
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys("bc1qgfam82qk7uwh5j2xxmcd8cmklpe0zackyj6r23")  # not in wallet
+        self.assertEqual("p2wpkh:L15oxP24NMNAXxq5r2aom24pHPtt3Fet8ZutgL155Bad93GSubM2",
+                         cmds.getprivatekeys("bc1q3g5tmkmlvxryhh843v4dz026avatc0zzr6h3af"))
+        # list of addresses tests
+        with self.assertRaises(Exception):
+            cmds.getprivatekeys(['bc1q3g5tmkmlvxryhh843v4dz026avatc0zzr6h3af', 'asd'])
+        self.assertEqual(['p2wpkh:L15oxP24NMNAXxq5r2aom24pHPtt3Fet8ZutgL155Bad93GSubM2', 'p2wpkh:L4rYY5QpfN6wJEF4SEKDpcGhTPnCe9zcGs6hiSnhpprZqVywFifN'],
+                         cmds.getprivatekeys(['bc1q3g5tmkmlvxryhh843v4dz026avatc0zzr6h3af', 'bc1q9pzjpjq4nqx5ycnywekcmycqz0wjp2nq604y2n']))
+
 
 class TestCommandsTestnet(TestCaseForTestnet):
 

electrum/tests/test_wallet.py

@@ -156,7 +156,8 @@ def test_create_new_wallet(self):
                               passphrase=passphrase,
                               password=password,
                               encrypt_file=encrypt_file,
-                              segwit=True)
+                              segwit=True,
+                              gap_limit=1)
         wallet = d['wallet']  # type: Standard_Wallet
         wallet.check_password(password)
         self.assertEqual(passphrase, wallet.keystore.get_passphrase(password))
@@ -173,7 +174,8 @@ def test_restore_wallet_from_text_mnemonic(self):
                                      network=None,
                                      passphrase=passphrase,
                                      password=password,
-                                     encrypt_file=encrypt_file)
+                                     encrypt_file=encrypt_file,
+                                     gap_limit=1)
         wallet = d['wallet']  # type: Standard_Wallet
         self.assertEqual(passphrase, wallet.keystore.get_passphrase(password))
         self.assertEqual(text, wallet.keystore.get_seed(password))
@@ -182,14 +184,14 @@ def test_restore_wallet_from_text_mnemonic(self):
 
     def test_restore_wallet_from_text_xpub(self):
         text = 'zpub6nydoME6CFdJtMpzHW5BNoPz6i6XbeT9qfz72wsRqGdgGEYeivso6xjfw8cGcCyHwF7BNW4LDuHF35XrZsovBLWMF4qXSjmhTXYiHbWqGLt'
-        d = restore_wallet_from_text(text, path=self.wallet_path, network=None)
+        d = restore_wallet_from_text(text, path=self.wallet_path, network=None, gap_limit=1)
         wallet = d['wallet']  # type: Standard_Wallet
         self.assertEqual(text, wallet.keystore.get_master_public_key())
         self.assertEqual('bc1q2ccr34wzep58d4239tl3x3734ttle92a8srmuw', wallet.get_receiving_addresses()[0])
 
     def test_restore_wallet_from_text_xprv(self):
         text = 'zprvAZzHPqhCMt51fskXBUYB1fTFYgG3CBjJUT4WEZTpGw6hPSDWBPZYZARC5sE9xAcX8NeWvvucFws8vZxEa65RosKAhy7r5MsmKTxr3hmNmea'
-        d = restore_wallet_from_text(text, path=self.wallet_path, network=None)
+        d = restore_wallet_from_text(text, path=self.wallet_path, network=None, gap_limit=1)
         wallet = d['wallet']  # type: Standard_Wallet
         self.assertEqual(text, wallet.keystore.get_master_private_key(password=None))
         self.assertEqual('bc1q2ccr34wzep58d4239tl3x3734ttle92a8srmuw', wallet.get_receiving_addresses()[0])

electrum/wallet.py

@@ -346,7 +346,9 @@ def get_redeem_script(self, address):
 
     def export_private_key(self, address, password):
         if self.is_watching_only():
-            return []
+            raise Exception(_("This is a watching-only wallet"))
+        if not self.is_mine(address):
+            raise Exception(_('Address not in wallet.') + f' {address}')
         index = self.get_address_index(address)
         pk, compressed = self.keystore.get_private_key(index, password)
         txin_type = self.get_txin_type(address)
@@ -1485,7 +1487,9 @@ def is_mine(self, address):
         return self.db.has_imported_address(address)
 
     def get_address_index(self, address):
-        # returns None is address is not mine
+        # returns None if address is not mine
+        if not is_address(address):
+            raise Exception(f"Invalid bitcoin address: {address}")
         return self.get_public_key(address)
 
     def get_public_key(self, address):
@@ -1677,6 +1681,8 @@ def is_beyond_limit(self, address):
         return True
 
     def get_address_index(self, address):
+        if not is_address(address):
+            raise Exception(f"Invalid bitcoin address: {address}")
         return self.db.get_address_index(address)
 
     def get_master_public_keys(self):
@@ -1875,7 +1881,7 @@ def wallet_class(wallet_type):
         raise WalletFileException("Unknown wallet type: " + str(wallet_type))
 
 
-def create_new_wallet(*, path, passphrase=None, password=None, encrypt_file=True, segwit=True):
+def create_new_wallet(*, path, passphrase=None, password=None, encrypt_file=True, segwit=True, gap_limit=None):
     """Create a new wallet"""
     storage = WalletStorage(path)
     if storage.file_exists():
@@ -1886,6 +1892,8 @@ def create_new_wallet(*, path, passphrase=None, password=None, encrypt_file=True
     k = keystore.from_seed(seed, passphrase)
     storage.put('keystore', k.dump())
     storage.put('wallet_type', 'standard')
+    if gap_limit is not None:
+        storage.put('gap_limit', gap_limit)
     wallet = Wallet(storage)
     wallet.update_password(old_pw=None, new_pw=password, encrypt_storage=encrypt_file)
     wallet.synchronize()
@@ -1896,7 +1904,8 @@ def create_new_wallet(*, path, passphrase=None, password=None, encrypt_file=True
 
 
 def restore_wallet_from_text(text, *, path, network=None,
-                             passphrase=None, password=None, encrypt_file=True):
+                             passphrase=None, password=None, encrypt_file=True,
+                             gap_limit=None):
     """Restore a wallet from text. Text can be a seed phrase, a master
     public key, a master private key, a list of bitcoin addresses
     or bitcoin private keys."""
@@ -1930,6 +1939,8 @@ def restore_wallet_from_text(text, *, path, network=None,
             raise Exception("Seed or key not recognized")
         storage.put('keystore', k.dump())
         storage.put('wallet_type', 'standard')
+        if gap_limit is not None:
+            storage.put('gap_limit', gap_limit)
         wallet = Wallet(storage)
 
     assert not storage.file_exists(), "file was created too soon! plaintext keys might have been written to disk"