Merge pull request #5272 from SomberNight/issue_4638_2

sweep/import key: disallow uncompressed segwit

electrum/base_wizard.py

@@ -199,7 +199,7 @@ class BaseWizard(object):
         self.choice_dialog(title=title, message=message, choices=choices, run_next=self.run)
 
     def import_addresses_or_keys(self):
-        v = lambda x: keystore.is_address_list(x) or keystore.is_private_key_list(x)
+        v = lambda x: keystore.is_address_list(x) or keystore.is_private_key_list(x, raise_on_error=True)
         title = _("Import Bitcoin Addresses")
         message = _("Enter a list of Bitcoin addresses (this will create a watching-only wallet), or a list of private keys.")
         self.add_xpub_dialog(title=title, message=message, run_next=self.on_import,

electrum/bitcoin.py

@@ -527,6 +527,9 @@ WIF_SCRIPT_TYPES = {
 WIF_SCRIPT_TYPES_INV = inv_dict(WIF_SCRIPT_TYPES)
 
 
+def is_segwit_script_type(txin_type: str) -> bool:
+    return txin_type in ('p2wpkh', 'p2wpkh-p2sh', 'p2wsh', 'p2wsh-p2sh')
+
 
 def serialize_privkey(secret: bytes, compressed: bool, txin_type: str,
                       internal_use: bool=False) -> str:
@@ -576,6 +579,10 @@ def deserialize_privkey(key: str) -> Tuple[str, bytes, bool]:
     if len(vch) not in [33, 34]:
         raise BitcoinException('invalid vch len for WIF key: {}'.format(len(vch)))
     compressed = len(vch) == 34
+
+    if is_segwit_script_type(txin_type) and not compressed:
+        raise BitcoinException('only compressed public keys can be used in segwit scripts')
+
     secret_bytes = vch[1:33]
     # we accept secrets outside curve range; cast into range here:
     secret_bytes = ecc.ECPrivkey.normalize_secret_bytes(secret_bytes)
@@ -615,11 +622,13 @@ def is_address(addr: str, *, net=None) -> bool:
            or is_b58_address(addr, net=net)
 
 
-def is_private_key(key: str) -> bool:
+def is_private_key(key: str, *, raise_on_error=False) -> bool:
     try:
-        k = deserialize_privkey(key)
-        return k is not False
-    except:
+        deserialize_privkey(key)
+        return True
+    except BaseException as e:
+        if raise_on_error:
+            raise
         return False
 
 

electrum/gui/kivy/uix/dialogs/installwizard.py

@@ -899,7 +899,12 @@ class AddXpubDialog(WizardDialog):
 
     def __init__(self, wizard, **kwargs):
         WizardDialog.__init__(self, wizard, **kwargs)
-        self.is_valid = kwargs['is_valid']
+        def is_valid(x):
+            try:
+                return kwargs['is_valid'](x)
+            except:
+                return False
+        self.is_valid = is_valid
         self.title = kwargs['title']
         self.message = kwargs['message']
         self.allow_multi = kwargs.get('allow_multi', False)

electrum/gui/qt/main_window.py

@@ -2670,14 +2670,22 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             if bitcoin.is_address(addr):
                 return addr
 
-        def get_pk():
+        def get_pk(*, raise_on_error=False):
             text = str(keys_e.toPlainText())
-            return keystore.get_private_keys(text)
+            return keystore.get_private_keys(text, raise_on_error=raise_on_error)
 
-        f = lambda: button.setEnabled(get_address() is not None and get_pk() is not None)
+        def on_edit():
+            valid_privkeys = False
+            try:
+                valid_privkeys = get_pk(raise_on_error=True) is not None
+            except Exception as e:
+                button.setToolTip(f'{_("Error")}: {str(e)}')
+            else:
+                button.setToolTip('')
+            button.setEnabled(get_address() is not None and valid_privkeys)
         on_address = lambda text: address_e.setStyleSheet((ColorScheme.DEFAULT if get_address() else ColorScheme.RED).as_stylesheet())
-        keys_e.textChanged.connect(f)
-        address_e.textChanged.connect(f)
+        keys_e.textChanged.connect(on_edit)
+        address_e.textChanged.connect(on_edit)
         address_e.textChanged.connect(on_address)
         on_address(str(address_e.text()))
         if not d.exec_():

electrum/gui/qt/seed_dialog.py

@@ -198,8 +198,14 @@ class KeysLayout(QVBoxLayout):
         return self.text_e.text()
 
     def on_edit(self):
-        b = self.is_valid(self.get_text())
-        self.parent.next_button.setEnabled(b)
+        valid = False
+        try:
+            valid = self.is_valid(self.get_text())
+        except Exception as e:
+            self.parent.next_button.setToolTip(f'{_("Error")}: {str(e)}')
+        else:
+            self.parent.next_button.setToolTip('')
+        self.parent.next_button.setEnabled(valid)
 
 
 class SeedDialog(WindowModalDialog):

electrum/keystore.py

@@ -752,19 +752,21 @@ def is_address_list(text):
     return bool(parts) and all(bitcoin.is_address(x) for x in parts)
 
 
-def get_private_keys(text, *, allow_spaces_inside_key=True):
+def get_private_keys(text, *, allow_spaces_inside_key=True, raise_on_error=False):
     if allow_spaces_inside_key:  # see #1612
         parts = text.split('\n')
         parts = map(lambda x: ''.join(x.split()), parts)
         parts = list(filter(bool, parts))
     else:
         parts = text.split()
-    if bool(parts) and all(bitcoin.is_private_key(x) for x in parts):
+    if bool(parts) and all(bitcoin.is_private_key(x, raise_on_error=raise_on_error) for x in parts):
         return parts
 
 
-def is_private_key_list(text, *, allow_spaces_inside_key=True):
-    return bool(get_private_keys(text, allow_spaces_inside_key=allow_spaces_inside_key))
+def is_private_key_list(text, *, allow_spaces_inside_key=True, raise_on_error=False):
+    return bool(get_private_keys(text,
+                                 allow_spaces_inside_key=allow_spaces_inside_key,
+                                 raise_on_error=raise_on_error))
 
 
 is_mpk = lambda x: is_old_mpk(x) or is_xpub(x)

electrum/plugins/ledger/ledger.py

@@ -4,7 +4,7 @@ import sys
 import traceback
 
 from electrum import ecc
-from electrum.bitcoin import TYPE_ADDRESS, int_to_hex, var_int
+from electrum.bitcoin import TYPE_ADDRESS, int_to_hex, var_int, is_segwit_script_type
 from electrum.bip32 import BIP32Node
 from electrum.i18n import _
 from electrum.keystore import Hardware_KeyStore
@@ -518,7 +518,7 @@ class Ledger_KeyStore(Hardware_KeyStore):
         client = self.get_client()
         address_path = self.get_derivation()[2:] + "/%d/%d"%sequence
         self.handler.show_message(_("Showing address ..."))
-        segwit = Transaction.is_segwit_inputtype(txin_type)
+        segwit = is_segwit_script_type(txin_type)
         segwitNative = txin_type == 'p2wpkh'
         try:
             client.getWalletPublicKey(address_path, showOnScreen=True, segwit=segwit, segwitNative=segwitNative)

electrum/tests/test_bitcoin.py

@@ -8,7 +8,7 @@ from electrum.bitcoin import (public_key_to_p2pkh, address_from_private_key,
                               is_b58_address, address_to_scripthash, is_minikey,
                               is_compressed_privkey, EncodeBase58Check, DecodeBase58Check,
                               script_num_to_hex, push_script, add_number_to_script, int_to_hex,
-                              opcodes, base_encode, base_decode)
+                              opcodes, base_encode, base_decode, BitcoinException)
 from electrum.bip32 import (BIP32Node, convert_bip32_intpath_to_strpath,
                             xpub_from_xprv, xpub_type, is_xprv, is_bip32_derivation,
                             is_xpub, convert_bip32_path_to_list_of_uint32,
@@ -736,6 +736,12 @@ class Test_keyImport(SequentialTestCase):
             self.assertEqual(priv_details['compressed'],
                              is_compressed_privkey(priv_details['priv']))
 
+    @needs_test_with_all_ecc_implementations
+    def test_segwit_uncompressed_pubkey(self):
+        with self.assertRaises(BitcoinException):
+            is_private_key("p2wpkh-p2sh:5JKXxT3wAZHcybJ9YNkuHur9vou6uuAnorBV9A8vVxGNFH5wvTW",
+                           raise_on_error=True)
+
 
 class TestBaseEncode(SequentialTestCase):
 

electrum/transaction.py

@@ -39,7 +39,7 @@ from .bitcoin import (TYPE_ADDRESS, TYPE_PUBKEY, TYPE_SCRIPT, hash_160,
                       hash160_to_p2sh, hash160_to_p2pkh, hash_to_segwit_addr,
                       hash_encode, var_int, TOTAL_COIN_SUPPLY_LIMIT_IN_BTC, COIN,
                       push_script, int_to_hex, push_script, b58_address_to_hash160,
-                      opcodes, add_number_to_script, base_decode)
+                      opcodes, add_number_to_script, base_decode, is_segwit_script_type)
 from .crypto import sha256d
 from .keystore import xpubkey_to_address, xpubkey_to_pubkey
 
@@ -815,11 +815,7 @@ class Transaction:
         if _type == 'address' and guess_for_address:
             _type = cls.guess_txintype_from_address(txin['address'])
         has_nonzero_witness = txin.get('witness', '00') not in ('00', None)
-        return cls.is_segwit_inputtype(_type) or has_nonzero_witness
-
-    @classmethod
-    def is_segwit_inputtype(cls, txin_type):
-        return txin_type in ('p2wpkh', 'p2wpkh-p2sh', 'p2wsh', 'p2wsh-p2sh')
+        return is_segwit_script_type(_type) or has_nonzero_witness
 
     @classmethod
     def guess_txintype_from_address(cls, addr):

electrum/wallet.py

@@ -1472,8 +1472,8 @@ class Imported_Wallet(Simple_Wallet):
         for key in keys:
             try:
                 txin_type, pubkey = self.keystore.import_privkey(key, password)
-            except Exception:
-                bad_keys.append((key, _('invalid private key')))
+            except Exception as e:
+                bad_keys.append((key, _('invalid private key') + f': {e}'))
                 continue
             if txin_type not in ('p2pkh', 'p2wpkh', 'p2wpkh-p2sh'):
                 bad_keys.append((key, _('not implemented type') + f': {txin_type}'))