network: catch untrusted exceptions from server in public methods

and re-raise a wrapper exception (that retains the original exc in a field)

closes #5111

electrum/network.py

@@ -43,7 +43,8 @@ from aiohttp import ClientResponse
 
 from . import util
 from .util import (PrintError, print_error, log_exceptions, ignore_exceptions,
-                   bfh, SilentTaskGroup, make_aiohttp_session, send_exception_to_crash_reporter)
+                   bfh, SilentTaskGroup, make_aiohttp_session, send_exception_to_crash_reporter,
+                   is_hash256_str, is_non_negative_integer)
 
 from .bitcoin import COIN
 from . import constants
@@ -195,6 +196,17 @@ class TxBroadcastUnknownError(TxBroadcastError):
                     _("Consider trying to connect to a different server, or updating Electrum."))
 
 
+class UntrustedServerReturnedError(Exception):
+    def __init__(self, *, original_exception):
+        self.original_exception = original_exception
+
+    def __str__(self):
+        return _("The server returned an error.")
+
+    def __repr__(self):
+        return f"<UntrustedServerReturnedError original_exception: {repr(self.original_exception)}>"
+
+
 INSTANCE = None
 
 
@@ -760,8 +772,21 @@ class Network(PrintError):
             raise BestEffortRequestFailed('no interface to do request on... gave up.')
         return make_reliable_wrapper
 
+    def catch_server_exceptions(func):
+        async def wrapper(self, *args, **kwargs):
+            try:
+                await func(self, *args, **kwargs)
+            except aiorpcx.jsonrpc.CodeMessageError as e:
+                raise UntrustedServerReturnedError(original_exception=e)
+        return wrapper
+
     @best_effort_reliable
+    @catch_server_exceptions
     async def get_merkle_for_transaction(self, tx_hash: str, tx_height: int) -> dict:
+        if not is_hash256_str(tx_hash):
+            raise Exception(f"{repr(tx_hash)} is not a txid")
+        if not is_non_negative_integer(tx_height):
+            raise Exception(f"{repr(tx_height)} is not a block height")
         return await self.interface.session.send_request('blockchain.transaction.get_merkle', [tx_hash, tx_height])
 
     @best_effort_reliable
@@ -919,24 +944,39 @@ class Network(PrintError):
         return _("Unknown error")
 
     @best_effort_reliable
-    async def request_chunk(self, height, tip=None, *, can_return_early=False):
+    @catch_server_exceptions
+    async def request_chunk(self, height: int, tip=None, *, can_return_early=False):
+        if not is_non_negative_integer(height):
+            raise Exception(f"{repr(height)} is not a block height")
         return await self.interface.request_chunk(height, tip=tip, can_return_early=can_return_early)
 
     @best_effort_reliable
+    @catch_server_exceptions
     async def get_transaction(self, tx_hash: str, *, timeout=None) -> str:
+        if not is_hash256_str(tx_hash):
+            raise Exception(f"{repr(tx_hash)} is not a txid")
         return await self.interface.session.send_request('blockchain.transaction.get', [tx_hash],
                                                          timeout=timeout)
 
     @best_effort_reliable
+    @catch_server_exceptions
     async def get_history_for_scripthash(self, sh: str) -> List[dict]:
+        if not is_hash256_str(sh):
+            raise Exception(f"{repr(sh)} is not a scripthash")
         return await self.interface.session.send_request('blockchain.scripthash.get_history', [sh])
 
     @best_effort_reliable
+    @catch_server_exceptions
     async def listunspent_for_scripthash(self, sh: str) -> List[dict]:
+        if not is_hash256_str(sh):
+            raise Exception(f"{repr(sh)} is not a scripthash")
         return await self.interface.session.send_request('blockchain.scripthash.listunspent', [sh])
 
     @best_effort_reliable
+    @catch_server_exceptions
     async def get_balance_for_scripthash(self, sh: str) -> dict:
+        if not is_hash256_str(sh):
+            raise Exception(f"{repr(sh)} is not a scripthash")
         return await self.interface.session.send_request('blockchain.scripthash.get_balance', [sh])
 
     def blockchain(self) -> Blockchain:

electrum/tests/test_util.py

@@ -1,6 +1,7 @@
 from decimal import Decimal
 
-from electrum.util import format_satoshis, format_fee_satoshis, parse_URI
+from electrum.util import (format_satoshis, format_fee_satoshis, parse_URI,
+                           is_hash256_str)
 
 from . import SequentialTestCase
 
@@ -93,3 +94,13 @@ class TestUtil(SequentialTestCase):
 
     def test_parse_URI_parameter_polution(self):
         self.assertRaises(Exception, parse_URI, 'bitcoin:15mKKb2eos1hWa6tisdPwwDC1a5J1y9nma?amount=0.0003&label=test&amount=30.0')
+
+    def test_is_hash256_str(self):
+        self.assertTrue(is_hash256_str('09a4c03e3bdf83bbe3955f907ee52da4fc12f4813d459bc75228b64ad08617c7'))
+        self.assertTrue(is_hash256_str('2A5C3F4062E4F2FCCE7A1C7B4310CB647B327409F580F4ED72CB8FC0B1804DFA'))
+        self.assertTrue(is_hash256_str('00' * 32))
+
+        self.assertFalse(is_hash256_str('00' * 33))
+        self.assertFalse(is_hash256_str('qweqwe'))
+        self.assertFalse(is_hash256_str(None))
+        self.assertFalse(is_hash256_str(7))

electrum/util.py

@@ -506,6 +506,26 @@ def is_valid_email(s):
     return re.match(regexp, s) is not None
 
 
+def is_hash256_str(text: str) -> bool:
+    if not isinstance(text, str): return False
+    if len(text) != 64: return False
+    try:
+        bytes.fromhex(text)
+    except:
+        return False
+    return True
+
+
+def is_non_negative_integer(val) -> bool:
+    try:
+        val = int(val)
+        if val >= 0:
+            return True
+    except:
+        pass
+    return False
+
+
 def format_satoshis_plain(x, decimal_point = 8):
     """Display a satoshi amount scaled.  Always uses a '.' as a decimal
     point and has no thousands separator"""

electrum/verifier.py

@@ -32,6 +32,7 @@ from .bitcoin import hash_decode, hash_encode
 from .transaction import Transaction
 from .blockchain import hash_header
 from .interface import GracefulDisconnect
+from .network import UntrustedServerReturnedError
 from . import constants
 
 if TYPE_CHECKING:
@@ -96,7 +97,9 @@ class SPV(NetworkJobOnDefaultServer):
     async def _request_and_verify_single_proof(self, tx_hash, tx_height):
         try:
             merkle = await self.network.get_merkle_for_transaction(tx_hash, tx_height)
-        except aiorpcx.jsonrpc.RPCError as e:
+        except UntrustedServerReturnedError as e:
+            if not isinstance(e.original_exception, aiorpcx.jsonrpc.RPCError):
+                raise
             self.print_error('tx {} not at height {}'.format(tx_hash, tx_height))
             self.wallet.remove_unverified_tx(tx_hash, tx_height)
             try: self.requested_merkle.remove(tx_hash)