Merge pull request #5011 from SomberNight/tx_broadcast_sanitize_response

network: sanitize tx broadcast response

electrum/commands.py

@@ -329,7 +329,8 @@ class Commands:
     def broadcast(self, tx):
         """Broadcast a transaction to the network. """
         tx = Transaction(tx)
-        return self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
+        self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
+        return tx.txid()
 
     @command('')
     def createmultisig(self, num, pubkeys):

electrum/gui/kivy/main_window.py

@@ -16,7 +16,7 @@ from electrum.plugin import run_hook
 from electrum.util import format_satoshis, format_satoshis_plain
 from electrum.paymentrequest import PR_UNPAID, PR_PAID, PR_UNKNOWN, PR_EXPIRED
 from electrum import blockchain
-from electrum.network import Network
+from electrum.network import Network, TxBroadcastError, BestEffortRequestFailed
 from .i18n import _
 
 from kivy.app import App
@@ -917,14 +917,16 @@ class ElectrumWindow(App):
         Clock.schedule_once(lambda dt: on_success(tx))
 
     def _broadcast_thread(self, tx, on_complete):
-
+        status = False
         try:
             self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
-        except Exception as e:
+        except TxBroadcastError as e:
-            ok, msg = False, repr(e)
+            msg = e.get_message_for_gui()
+        except BestEffortRequestFailed as e:
+            msg = repr(e)
         else:
-            ok, msg = True, tx.txid()
+            status, msg = True, tx.txid()
-        Clock.schedule_once(lambda dt: on_complete(ok, msg))
+        Clock.schedule_once(lambda dt: on_complete(status, msg))
 
     def broadcast(self, tx, pr=None):
         def on_complete(ok, msg):
@@ -937,11 +939,8 @@ class ElectrumWindow(App):
                     self.wallet.invoices.save()
                     self.update_tab('invoices')
             else:
-                display_msg = _('The server returned an error when broadcasting the transaction.')
+                msg = msg or ''
-                if msg:
+                self.show_error(msg)
-                    display_msg += '\n' + msg
-                display_msg = display_msg[:500]
-                self.show_error(display_msg)
 
         if self.network and self.network.is_connected():
             self.show_info(_('Sending'))

electrum/gui/qt/main_window.py

@@ -62,7 +62,7 @@ from electrum.address_synchronizer import AddTransactionException
 from electrum.wallet import (Multisig_Wallet, CannotBumpFee, Abstract_Wallet,
                              sweep_preparations, InternalAddressCorruption)
 from electrum.version import ELECTRUM_VERSION
-from electrum.network import Network
+from electrum.network import Network, TxBroadcastError, BestEffortRequestFailed
 from electrum.exchange_rate import FxThread
 from electrum.simple_config import SimpleConfig
 
@@ -1660,10 +1660,13 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
             if pr and pr.has_expired():
                 self.payment_request = None
                 return False, _("Payment request has expired")
+            status = False
             try:
                 self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
-            except Exception as e:
+            except TxBroadcastError as e:
-                status, msg = False, repr(e)
+                msg = e.get_message_for_gui()
+            except BestEffortRequestFailed as e:
+                msg = repr(e)
             else:
                 status, msg = True, tx.txid()
             if pr and status is True:
@@ -1691,10 +1694,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, PrintError):
                     self.invoice_list.update()
                     self.do_clear()
                 else:
-                    display_msg = _('The server returned an error when broadcasting the transaction.')
+                    msg = msg or ''
-                    if msg:
+                    parent.show_error(msg)
-                        display_msg += '\n' + msg
-                    parent.show_error(display_msg)
 
         WaitingDialog(self, _('Broadcasting transaction...'),
                       broadcast_thread, broadcast_done, self.on_error)

electrum/gui/stdio.py

@@ -6,6 +6,7 @@ from electrum import WalletStorage, Wallet
 from electrum.util import format_satoshis, set_verbosity
 from electrum.bitcoin import is_address, COIN, TYPE_ADDRESS
 from electrum.transaction import TxOutput
+from electrum.network import TxBroadcastError, BestEffortRequestFailed
 
 _ = lambda x:x  # i18n
 
@@ -205,10 +206,12 @@ class ElectrumGui:
         print(_("Please wait..."))
         try:
             self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
-        except Exception as e:
+        except TxBroadcastError as e:
-            display_msg = _('The server returned an error when broadcasting the transaction.')
+            msg = e.get_message_for_gui()
-            display_msg += '\n' + repr(e)
+            print(msg)
-            print(display_msg)
+        except BestEffortRequestFailed as e:
+            msg = repr(e)
+            print(msg)
         else:
             print(_('Payment sent.'))
             #self.do_clear()

electrum/gui/text.py

@@ -12,7 +12,7 @@ from electrum.bitcoin import is_address, COIN, TYPE_ADDRESS
 from electrum.transaction import TxOutput
 from electrum.wallet import Wallet
 from electrum.storage import WalletStorage
-from electrum.network import NetworkParameters
+from electrum.network import NetworkParameters, TxBroadcastError, BestEffortRequestFailed
 from electrum.interface import deserialize_server
 
 _ = lambda x:x  # i18n
@@ -369,10 +369,12 @@ class ElectrumGui:
         self.show_message(_("Please wait..."), getchar=False)
         try:
             self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
-        except Exception as e:
+        except TxBroadcastError as e:
-            display_msg = _('The server returned an error when broadcasting the transaction.')
+            msg = e.get_message_for_gui()
-            display_msg += '\n' + repr(e)
+            self.show_message(msg)
-            self.show_message(display_msg)
+        except BestEffortRequestFailed as e:
+            msg = repr(e)
+            self.show_message(msg)
         else:
             self.show_message(_('Payment sent.'))
             self.do_clear()

electrum/network.py

@@ -37,6 +37,7 @@ import traceback
 
 import dns
 import dns.resolver
+import aiorpcx
 from aiorpcx import TaskGroup
 from aiohttp import ClientResponse
 
@@ -53,6 +54,7 @@ from .interface import (Interface, serialize_server, deserialize_server,
                         RequestTimedOut, NetworkTimeout)
 from .version import PROTOCOL_VERSION
 from .simple_config import SimpleConfig
+from .i18n import _
 
 NODES_RETRY_INTERVAL = 60
 SERVER_RETRY_INTERVAL = 10
@@ -162,6 +164,30 @@ def deserialize_proxy(s: str) -> Optional[dict]:
     return proxy
 
 
+class BestEffortRequestFailed(Exception): pass
+
+
+class TxBroadcastError(Exception):
+    def get_message_for_gui(self):
+        raise NotImplementedError()
+
+
+class TxBroadcastHashMismatch(TxBroadcastError):
+    def get_message_for_gui(self):
+        return "{}\n{}\n\n{}" \
+            .format(_("The server returned an unexpected transaction ID when broadcasting the transaction."),
+                    _("Consider trying to connect to a different server, or updating Electrum."),
+                    str(self))
+
+
+class TxBroadcastServerReturnedError(TxBroadcastError):
+    def get_message_for_gui(self):
+        return "{}\n{}\n\n{}" \
+            .format(_("The server returned an error when broadcasting the transaction."),
+                    _("Consider trying to connect to a different server, or updating Electrum."),
+                    str(self))
+
+
 INSTANCE = None
 
 
@@ -724,7 +750,7 @@ class Network(PrintError):
                             continue  # try again
                     return success_fut.result()
                 # otherwise; try again
-            raise Exception('no interface to do request on... gave up.')
+            raise BestEffortRequestFailed('no interface to do request on... gave up.')
         return make_reliable_wrapper
 
     @best_effort_reliable
@@ -732,14 +758,152 @@ class Network(PrintError):
         return await self.interface.session.send_request('blockchain.transaction.get_merkle', [tx_hash, tx_height])
 
     @best_effort_reliable
-    async def broadcast_transaction(self, tx, *, timeout=None):
+    async def broadcast_transaction(self, tx, *, timeout=None) -> None:
         if timeout is None:
             timeout = self.get_network_timeout_seconds(NetworkTimeout.Urgent)
-        out = await self.interface.session.send_request('blockchain.transaction.broadcast', [str(tx)], timeout=timeout)
+        try:
+            out = await self.interface.session.send_request('blockchain.transaction.broadcast', [str(tx)], timeout=timeout)
+            # note: both 'out' and exception messages are untrusted input from the server
+        except aiorpcx.jsonrpc.RPCError as e:
+            self.print_error(f"broadcast_transaction error: {repr(e)}")
+            raise TxBroadcastServerReturnedError(self.sanitize_tx_broadcast_response(e.message)) from e
         if out != tx.txid():
-            # note: this is untrusted input from the server
+            self.print_error(f"unexpected txid for broadcast_transaction: {out} != {tx.txid()}")
-            raise Exception(out)
+            raise TxBroadcastHashMismatch(_("Server returned unexpected transaction ID."))
-        return out  # txid
+
+    @staticmethod
+    def sanitize_tx_broadcast_response(server_msg) -> str:
+        # Unfortunately, bitcoind and hence the Electrum protocol doesn't return a useful error code.
+        # So, we use substring matching to grok the error message.
+        # server_msg is untrusted input so it should not be shown to the user. see #4968
+        server_msg = str(server_msg)
+        server_msg = server_msg.replace("\n", r"\n")
+        # https://github.com/bitcoin/bitcoin/blob/cd42553b1178a48a16017eff0b70669c84c3895c/src/policy/policy.cpp
+        # grep "reason ="
+        policy_error_messages = {
+            r"version": None,
+            r"tx-size": _("The transaction was rejected because it is too large."),
+            r"scriptsig-size": None,
+            r"scriptsig-not-pushonly": None,
+            r"scriptpubkey": None,
+            r"bare-multisig": None,
+            r"dust": _("Transaction could not be broadcast due to dust outputs."),
+            r"multi-op-return": _("The transaction was rejected because it contains more than 1 OP_RETURN input."),
+        }
+        for substring in policy_error_messages:
+            if substring in server_msg:
+                msg = policy_error_messages[substring]
+                return msg if msg else substring
+        # https://github.com/bitcoin/bitcoin/blob/cd42553b1178a48a16017eff0b70669c84c3895c/src/script/script_error.cpp
+        script_error_messages = {
+            r"Script evaluated without error but finished with a false/empty top stack element",
+            r"Script failed an OP_VERIFY operation",
+            r"Script failed an OP_EQUALVERIFY operation",
+            r"Script failed an OP_CHECKMULTISIGVERIFY operation",
+            r"Script failed an OP_CHECKSIGVERIFY operation",
+            r"Script failed an OP_NUMEQUALVERIFY operation",
+            r"Script is too big",
+            r"Push value size limit exceeded",
+            r"Operation limit exceeded",
+            r"Stack size limit exceeded",
+            r"Signature count negative or greater than pubkey count",
+            r"Pubkey count negative or limit exceeded",
+            r"Opcode missing or not understood",
+            r"Attempted to use a disabled opcode",
+            r"Operation not valid with the current stack size",
+            r"Operation not valid with the current altstack size",
+            r"OP_RETURN was encountered",
+            r"Invalid OP_IF construction",
+            r"Negative locktime",
+            r"Locktime requirement not satisfied",
+            r"Signature hash type missing or not understood",
+            r"Non-canonical DER signature",
+            r"Data push larger than necessary",
+            r"Only non-push operators allowed in signatures",
+            r"Non-canonical signature: S value is unnecessarily high",
+            r"Dummy CHECKMULTISIG argument must be zero",
+            r"OP_IF/NOTIF argument must be minimal",
+            r"Signature must be zero for failed CHECK(MULTI)SIG operation",
+            r"NOPx reserved for soft-fork upgrades",
+            r"Witness version reserved for soft-fork upgrades",
+            r"Public key is neither compressed or uncompressed",
+            r"Extra items left on stack after execution",
+            r"Witness program has incorrect length",
+            r"Witness program was passed an empty witness",
+            r"Witness program hash mismatch",
+            r"Witness requires empty scriptSig",
+            r"Witness requires only-redeemscript scriptSig",
+            r"Witness provided for non-witness script",
+            r"Using non-compressed keys in segwit",
+            r"Using OP_CODESEPARATOR in non-witness script",
+            r"Signature is found in scriptCode",
+        }
+        for substring in script_error_messages:
+            if substring in server_msg:
+                return substring
+        # https://github.com/bitcoin/bitcoin/blob/cd42553b1178a48a16017eff0b70669c84c3895c/src/validation.cpp
+        # grep "REJECT_"
+        # should come after script_error.cpp (due to e.g. non-mandatory-script-verify-flag)
+        validation_error_messages = {
+            r"coinbase",
+            r"tx-size-small",
+            r"non-final",
+            r"txn-already-in-mempool",
+            r"txn-mempool-conflict",
+            r"txn-already-known",
+            r"non-BIP68-final",
+            r"bad-txns-nonstandard-inputs",
+            r"bad-witness-nonstandard",
+            r"bad-txns-too-many-sigops",
+            r"mempool min fee not met",
+            r"min relay fee not met",
+            r"absurdly-high-fee",
+            r"too-long-mempool-chain",
+            r"bad-txns-spends-conflicting-tx",
+            r"insufficient fee",
+            r"too many potential replacements",
+            r"replacement-adds-unconfirmed",
+            r"mempool full",
+            r"non-mandatory-script-verify-flag",
+            r"mandatory-script-verify-flag-failed",
+        }
+        for substring in validation_error_messages:
+            if substring in server_msg:
+                return substring
+        # https://github.com/bitcoin/bitcoin/blob/cd42553b1178a48a16017eff0b70669c84c3895c/src/rpc/rawtransaction.cpp
+        # grep "RPC_TRANSACTION"
+        # grep "RPC_DESERIALIZATION_ERROR"
+        rawtransaction_error_messages = {
+            r"Missing inputs",
+            r"transaction already in block chain",
+            r"TX decode failed",
+        }
+        for substring in rawtransaction_error_messages:
+            if substring in server_msg:
+                return substring
+        # https://github.com/bitcoin/bitcoin/blob/cd42553b1178a48a16017eff0b70669c84c3895c/src/consensus/tx_verify.cpp
+        # grep "REJECT_"
+        tx_verify_error_messages = {
+            r"bad-txns-vin-empty",
+            r"bad-txns-vout-empty",
+            r"bad-txns-oversize",
+            r"bad-txns-vout-negative",
+            r"bad-txns-vout-toolarge",
+            r"bad-txns-txouttotal-toolarge",
+            r"bad-txns-inputs-duplicate",
+            r"bad-cb-length",
+            r"bad-txns-prevout-null",
+            r"bad-txns-inputs-missingorspent",
+            r"bad-txns-premature-spend-of-coinbase",
+            r"bad-txns-inputvalues-outofrange",
+            r"bad-txns-in-belowout",
+            r"bad-txns-fee-outofrange",
+        }
+        for substring in tx_verify_error_messages:
+            if substring in server_msg:
+                return substring
+        # otherwise:
+        return _("Unknown error")
 
     @best_effort_reliable
     async def request_chunk(self, height, tip=None, *, can_return_early=False):