diff --git a/electrum/base_crash_reporter.py b/electrum/base_crash_reporter.py index b2c95be80..ed75a1ac6 100644 --- a/electrum/base_crash_reporter.py +++ b/electrum/base_crash_reporter.py @@ -30,7 +30,7 @@ from typing import NamedTuple, Optional from .version import ELECTRUM_VERSION from . import constants from .i18n import _ -from .util import make_aiohttp_session +from .util import make_aiohttp_session, error_text_str_to_safe_str from .logging import describe_os_version, Logger, get_git_version @@ -80,7 +80,8 @@ class BaseCrashReporter(Logger): report = json.dumps(report) coro = self.do_post(proxy, BaseCrashReporter.report_server + "/crash.json", data=report) response = asyncio.run_coroutine_threadsafe(coro, asyncio_loop).result(timeout) - self.logger.info(f"Crash report sent. Got response [DO NOT TRUST THIS MESSAGE]: {response!r}") + self.logger.info( + f"Crash report sent. Got response [DO NOT TRUST THIS MESSAGE]: {error_text_str_to_safe_str(response)}") response = json.loads(response) assert isinstance(response, dict), type(response) # sanitize URL @@ -98,7 +99,7 @@ class BaseCrashReporter(Logger): ) return ret - async def do_post(self, proxy, url, data): + async def do_post(self, proxy, url, data) -> str: async with make_aiohttp_session(proxy) as session: async with session.post(url, data=data, raise_for_status=True) as resp: return await resp.text() diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 1110e1726..c4f2a3827 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -22,7 +22,7 @@ from . import ecc from .ecc import sig_string_from_r_and_s, der_sig_from_sig_string from . import constants from .util import (bfh, log_exceptions, ignore_exceptions, chunks, OldTaskGroup, - UnrelatedTransactionException) + UnrelatedTransactionException, error_text_bytes_to_safe_str) from . import transaction from .bitcoin import make_op_return from .transaction import PartialTxOutput, match_script_against_template, Sighash @@ -241,13 +241,14 @@ class Peer(Logger): def on_warning(self, payload): chan_id = payload.get("channel_id") + err_bytes = payload['data'] self.logger.info(f"remote peer sent warning [DO NOT TRUST THIS MESSAGE]: " - f"{payload['data'].decode('ascii')}. chan_id={chan_id.hex()}") + f"{error_text_bytes_to_safe_str(err_bytes)}. chan_id={chan_id.hex()}") if chan_id in self.channels: - self.ordered_message_queues[chan_id].put_nowait((None, {'warning': payload['data']})) + self.ordered_message_queues[chan_id].put_nowait((None, {'warning': err_bytes})) elif chan_id in self.temp_id_to_id: chan_id = self.temp_id_to_id[chan_id] or chan_id - self.ordered_message_queues[chan_id].put_nowait((None, {'warning': payload['data']})) + self.ordered_message_queues[chan_id].put_nowait((None, {'warning': err_bytes})) else: # if no existing channel is referred to by channel_id: # - MUST ignore the message. @@ -256,20 +257,21 @@ class Peer(Logger): def on_error(self, payload): chan_id = payload.get("channel_id") + err_bytes = payload['data'] self.logger.info(f"remote peer sent error [DO NOT TRUST THIS MESSAGE]: " - f"{payload['data'].decode('ascii')}. chan_id={chan_id.hex()}") + f"{error_text_bytes_to_safe_str(err_bytes)}. chan_id={chan_id.hex()}") if chan_id in self.channels: self.schedule_force_closing(chan_id) - self.ordered_message_queues[chan_id].put_nowait((None, {'error': payload['data']})) + self.ordered_message_queues[chan_id].put_nowait((None, {'error': err_bytes})) elif chan_id in self.temp_id_to_id: chan_id = self.temp_id_to_id[chan_id] or chan_id - self.ordered_message_queues[chan_id].put_nowait((None, {'error': payload['data']})) + self.ordered_message_queues[chan_id].put_nowait((None, {'error': err_bytes})) elif chan_id == bytes(32): # if channel_id is all zero: # - MUST fail all channels with the sending node. for cid in self.channels: self.schedule_force_closing(cid) - self.ordered_message_queues[cid].put_nowait((None, {'error': payload['data']})) + self.ordered_message_queues[cid].put_nowait((None, {'error': err_bytes})) else: # if no existing channel is referred to by channel_id: # - MUST ignore the message. @@ -337,12 +339,11 @@ class Peer(Logger): q = self.ordered_message_queues[channel_id] name, payload = await asyncio.wait_for(q.get(), LN_P2P_NETWORK_TIMEOUT) # raise exceptions for errors/warnings, so that the caller sees them - if payload.get('error'): + if (err_bytes := (payload.get("error") or payload.get("warning"))) is not None: + err_type = "error" if payload.get("error") else "warning" + err_text = error_text_bytes_to_safe_str(err_bytes) raise GracefulDisconnect( - f"remote peer sent error [DO NOT TRUST THIS MESSAGE]: {payload['error'].decode('ascii')}") - elif payload.get('warning'): - raise GracefulDisconnect( - f"remote peer sent warning [DO NOT TRUST THIS MESSAGE]: {payload['warning'].decode('ascii')}") + f"remote peer sent {err_type} [DO NOT TRUST THIS MESSAGE]: {err_text}") if name != expected_name: raise Exception(f"Received unexpected '{name}'") return payload diff --git a/electrum/network.py b/electrum/network.py index 71467d048..1a3ffebc1 100644 --- a/electrum/network.py +++ b/electrum/network.py @@ -46,7 +46,7 @@ from . import util from .util import (log_exceptions, ignore_exceptions, OldTaskGroup, bfh, make_aiohttp_session, send_exception_to_crash_reporter, is_hash256_str, is_non_negative_integer, MyEncoder, NetworkRetryManager, - nullcontext) + nullcontext, error_text_str_to_safe_str) from .bitcoin import COIN from . import constants from . import blockchain @@ -235,8 +235,9 @@ class UntrustedServerReturnedError(NetworkException): return _("The server returned an error.") def __repr__(self): + e = self.original_exception return (f"") + f"[DO NOT TRUST THIS MESSAGE] original_exception: {error_text_str_to_safe_str(repr(e))}>") _INSTANCE = None @@ -924,14 +925,15 @@ class Network(Logger, NetworkRetryManager[ServerAddr]): except (RequestTimedOut, asyncio.CancelledError, asyncio.TimeoutError): raise # pass-through except aiorpcx.jsonrpc.CodeMessageError as e: - self.logger.info(f"broadcast_transaction error [DO NOT TRUST THIS MESSAGE]: {repr(e)}") + self.logger.info(f"broadcast_transaction error [DO NOT TRUST THIS MESSAGE]: {error_text_str_to_safe_str(repr(e))}") raise TxBroadcastServerReturnedError(self.sanitize_tx_broadcast_response(e.message)) from e except BaseException as e: # intentional BaseException for sanity! - self.logger.info(f"broadcast_transaction error2 [DO NOT TRUST THIS MESSAGE]: {repr(e)}") + self.logger.info(f"broadcast_transaction error2 [DO NOT TRUST THIS MESSAGE]: {error_text_str_to_safe_str(repr(e))}") send_exception_to_crash_reporter(e) raise TxBroadcastUnknownError() from e if out != tx.txid(): - self.logger.info(f"unexpected txid for broadcast_transaction [DO NOT TRUST THIS MESSAGE]: {out} != {tx.txid()}") + self.logger.info(f"unexpected txid for broadcast_transaction [DO NOT TRUST THIS MESSAGE]: " + f"{error_text_str_to_safe_str(out)} != {tx.txid()}") raise TxBroadcastHashMismatch(_("Server returned unexpected transaction ID.")) async def try_broadcasting(self, tx, name) -> bool: diff --git a/electrum/paymentrequest.py b/electrum/paymentrequest.py index afc876e20..8d46909e2 100644 --- a/electrum/paymentrequest.py +++ b/electrum/paymentrequest.py @@ -39,7 +39,7 @@ except ImportError: sys.exit("Error: could not find paymentrequest_pb2.py. Create it with 'contrib/generate_payreqpb2.sh'") from . import bitcoin, constants, ecc, util, transaction, x509, rsakey -from .util import bfh, make_aiohttp_session +from .util import bfh, make_aiohttp_session, error_text_bytes_to_safe_str from .invoices import Invoice, get_id_from_onchain_outputs from .crypto import sha256 from .bitcoin import address_to_script @@ -94,12 +94,8 @@ async def get_payment_request(url: str) -> 'PaymentRequest': if isinstance(e, aiohttp.ClientResponseError): error += f"\nGot HTTP status code {e.status}." if resp_content: - try: - error_text_received = resp_content.decode("utf8") - except UnicodeDecodeError: - error_text_received = "(failed to decode error)" - else: - error_text_received = error_text_received[:400] + error_text_received = error_text_bytes_to_safe_str(resp_content) + error_text_received = error_text_received[:400] error_oneline = ' -- '.join(error.split('\n')) _logger.info(f"{error_oneline} -- [DO NOT TRUST THIS MESSAGE] " f"{repr(e)} text: {error_text_received}") @@ -306,12 +302,8 @@ class PaymentRequest: if isinstance(e, aiohttp.ClientResponseError): error += f"\nGot HTTP status code {e.status}." if resp_content: - try: - error_text_received = resp_content.decode("utf8") - except UnicodeDecodeError: - error_text_received = "(failed to decode error)" - else: - error_text_received = error_text_received[:400] + error_text_received = error_text_bytes_to_safe_str(resp_content) + error_text_received = error_text_received[:400] error_oneline = ' -- '.join(error.split('\n')) _logger.info(f"{error_oneline} -- [DO NOT TRUST THIS MESSAGE] " f"{repr(e)} text: {error_text_received}") diff --git a/electrum/tests/test_util.py b/electrum/tests/test_util.py index 60659d237..9668aa2e1 100644 --- a/electrum/tests/test_util.py +++ b/electrum/tests/test_util.py @@ -344,3 +344,26 @@ class TestUtil(ElectrumTestCase): self.assertFalse(util.is_subpath("/a/b/c", "c")) self.assertFalse(util.is_subpath("a", "/a/b/c")) self.assertFalse(util.is_subpath("c", "/a/b/c")) + + def test_error_text_bytes_to_safe_str(self): + # ascii + self.assertEqual("'test'", util.error_text_bytes_to_safe_str(b"test")) + self.assertEqual('"test123 \'QWE"', util.error_text_bytes_to_safe_str(b"test123 'QWE")) + self.assertEqual("'prefix: \\x08\\x08\\x08\\x08\\x08\\x08\\x08\\x08malicious_stuff'", + util.error_text_bytes_to_safe_str(b"prefix: " + 8 * b"\x08" + b"malicious_stuff")) + # unicode + self.assertEqual("'here is some unicode: \\\\xe2\\\\x82\\\\xbf \\\\xf0\\\\x9f\\\\x98\\\\x80 \\\\xf0\\\\x9f\\\\x98\\\\x88'", + util.error_text_bytes_to_safe_str(b'here is some unicode: \xe2\x82\xbf \xf0\x9f\x98\x80 \xf0\x9f\x98\x88')) + # not even unicode + self.assertEqual("""\'\\x00\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x0b\\x0c\\r\\x0e\\x0f\\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17\\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f !"#$%&\\\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\\x7f\\\\x80\\\\x81\\\\x82\\\\x83\\\\x84\\\\x85\\\\x86\\\\x87\\\\x88\\\\x89\\\\x8a\\\\x8b\\\\x8c\\\\x8d\\\\x8e\\\\x8f\\\\x90\\\\x91\\\\x92\\\\x93\\\\x94\\\\x95\\\\x96\\\\x97\\\\x98\\\\x99\\\\x9a\\\\x9b\\\\x9c\\\\x9d\\\\x9e\\\\x9f\\\\xa0\\\\xa1\\\\xa2\\\\xa3\\\\xa4\\\\xa5\\\\xa6\\\\xa7\\\\xa8\\\\xa9\\\\xaa\\\\xab\\\\xac\\\\xad\\\\xae\\\\xaf\\\\xb0\\\\xb1\\\\xb2\\\\xb3\\\\xb4\\\\xb5\\\\xb6\\\\xb7\\\\xb8\\\\xb9\\\\xba\\\\xbb\\\\xbc\\\\xbd\\\\xbe\\\\xbf\\\\xc0\\\\xc1\\\\xc2\\\\xc3\\\\xc4\\\\xc5\\\\xc6\\\\xc7\\\\xc8\\\\xc9\\\\xca\\\\xcb\\\\xcc\\\\xcd\\\\xce\\\\xcf\\\\xd0\\\\xd1\\\\xd2\\\\xd3\\\\xd4\\\\xd5\\\\xd6\\\\xd7\\\\xd8\\\\xd9\\\\xda\\\\xdb\\\\xdc\\\\xdd\\\\xde\\\\xdf\\\\xe0\\\\xe1\\\\xe2\\\\xe3\\\\xe4\\\\xe5\\\\xe6\\\\xe7\\\\xe8\\\\xe9\\\\xea\\\\xeb\\\\xec\\\\xed\\\\xee\\\\xef\\\\xf0\\\\xf1\\\\xf2\\\\xf3\\\\xf4\\\\xf5\\\\xf6\\\\xf7\\\\xf8\\\\xf9\\\\xfa\\\\xfb\\\\xfc\\\\xfd\\\\xfe\\\\xff\'""", + util.error_text_bytes_to_safe_str(bytes(range(256)))) + + def test_error_text_str_to_safe_str(self): + # ascii + self.assertEqual("'test'", util.error_text_str_to_safe_str("test")) + self.assertEqual('"test123 \'QWE"', util.error_text_str_to_safe_str("test123 'QWE")) + self.assertEqual("'prefix: \\x08\\x08\\x08\\x08\\x08\\x08\\x08\\x08malicious_stuff'", + util.error_text_str_to_safe_str("prefix: " + 8 * "\x08" + "malicious_stuff")) + # unicode + self.assertEqual("'here is some unicode: \\\\u20bf \\\\U0001f600 \\\\U0001f608'", + util.error_text_str_to_safe_str("here is some unicode: ₿ 😀 😈")) diff --git a/electrum/util.py b/electrum/util.py index 9ad8fbb0f..742f873a1 100644 --- a/electrum/util.py +++ b/electrum/util.py @@ -2059,3 +2059,26 @@ def get_running_loop() -> Optional[asyncio.AbstractEventLoop]: return asyncio.get_running_loop() except RuntimeError: return None + + +def error_text_str_to_safe_str(err: str) -> str: + """Converts an untrusted error string to a sane printable ascii str. + Never raises. + """ + return error_text_bytes_to_safe_str(err.encode("ascii", errors='backslashreplace')) + + +def error_text_bytes_to_safe_str(err: bytes) -> str: + """Converts an untrusted error bytes text to a sane printable ascii str. + Never raises. + + Note that naive ascii conversion would be insufficient. Fun stuff: + >>> b = b"my_long_prefix_blabla" + 21 * b"\x08" + b"malicious_stuff" + >>> s = b.decode("ascii") + >>> print(s) + malicious_stuffblabla + """ + # convert to ascii, to get rid of unicode stuff + ascii_text = err.decode("ascii", errors='backslashreplace') + # do repr to handle ascii special chars (especially when printing/logging the str) + return repr(ascii_text)