diff --git a/electrum/json_db.py b/electrum/json_db.py index 28d1ec141..34a74e259 100644 --- a/electrum/json_db.py +++ b/electrum/json_db.py @@ -28,6 +28,7 @@ import json from typing import TYPE_CHECKING, Optional, Sequence, List, Union import jsonpatch +import jsonpointer from . import util from .util import WalletFileException, profiler @@ -36,6 +37,15 @@ from .logging import Logger if TYPE_CHECKING: from .storage import WalletStorage + +# We monkeypatch exceptions in the jsonpatch package to ensure they do not contain secrets from the DB. +# We often log exceptions and offer to send them to the crash reporter, so they must not contain secrets. +jsonpointer.JsonPointerException.__str__ = lambda self: """(JPE) 'redacted'""" +jsonpointer.JsonPointerException.__repr__ = lambda self: """""" +jsonpatch.JsonPatchException.__str__ = lambda self: """(JPE) 'redacted'""" +jsonpatch.JsonPatchException.__repr__ = lambda self: """""" + + def modifier(func): def wrapper(self, *args, **kwargs): with self.lock: diff --git a/tests/test_jsondb.py b/tests/test_jsondb.py new file mode 100644 index 000000000..83444d368 --- /dev/null +++ b/tests/test_jsondb.py @@ -0,0 +1,88 @@ +import contextlib +import copy +import traceback + +import jsonpatch +from jsonpatch import JsonPatchException +from jsonpointer import JsonPointerException + +from . import ElectrumTestCase + + +class TestJsonpatch(ElectrumTestCase): + + async def test_op_replace(self): + data1 = {'foo': 'bar', 'numbers': [1, 3, 4, 8], 'dictlevelA1': {'secret1': 2, 'secret2': 4, 'secret3': 6}} + patches = [{"op": "replace", "path": "/dictlevelA1/secret2", "value": 2222}] + jpatch = jsonpatch.JsonPatch(patches) + data2 = jpatch.apply(data1) + self.assertEqual( + {'foo': 'bar', 'numbers': [1, 3, 4, 8], 'dictlevelA1': {'secret1': 2, 'secret2': 2222, 'secret3': 6}}, + data2 + ) + + @contextlib.contextmanager + def _customAssertRaises(self, *args, **kwargs): + with self.assertRaises(*args, **kwargs) as ctx: + try: + yield ctx + except Exception as e: + # save original traceback now, as assertRaises will destroy most of it imminently: + ctx._customctx_original_tb = "".join(traceback.format_exception(e)) + raise + + async def test_patch_does_not_leak_privatekeys(self): + data1 = { + 'dictlevelB1': 'secret77', + 'dictlevelC1': [1, "secret99", 4, 8], + 'dictlevelA1': {"dictlevelA2_aa": "secret11", "dictlevelA2_bb": "secret12", "dictlevelA2_cc": "secret13"}} + def fail_if_leaking_secret(ctx) -> None: + self.assertNotIn("secret", str(ctx.exception)) + self.assertNotIn("secret", repr(ctx.exception)) + self.assertIn("redacted", str(ctx.exception)) # injected by our monkeypatching + self.assertIn("redacted", repr(ctx.exception)) # injected by our monkeypatching + self.assertNotIn("secret", ctx._customctx_original_tb) + # Note, crucially, the following assert would FAIL: + # That is, exceptions might "leak" the db *path* but not values stored at the innermost level. + # IOW, in case of dicts, secrets should be stored in values. Dict keys should never contain secrets, + # as dict keys can appear in tracebacks. + #self.assertNotIn("dictlevel", ctx._customctx_original_tb) + # op "replace" + with self.subTest(msg="replace_dict_inner_key_missing"): + patches = [{"op": "replace", "path": "/dictlevelA1/dictlevelX2", "value": "nakamoto_secret"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPatchException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx) + with self.subTest(msg="replace_dict_outer_key_missing"): + patches = [{"op": "replace", "path": "/dictlevelX1/dictlevelX2", "value": "nakamoto_secret"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPointerException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx) + # op "remove" + with self.subTest(msg="remove_dict_inner_key_missing"): + patches = [{"op": "remove", "path": "/dictlevelA1/dictlevelX2"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPatchException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx) + with self.subTest(msg="remove_dict_outer_key_missing"): + patches = [{"op": "remove", "path": "/dictlevelX1/dictlevelX2"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPointerException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx) + # op "add" + with self.subTest(msg="add_dict_inner_key_missing"): + patches = [{"op": "add", "path": "/dictlevelA1/dictlevelX2/dictlevelX3/dictlevelX4", "value": "nakamoto_secret"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPointerException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx) + with self.subTest(msg="add_dict_outer_key_missing"): + patches = [{"op": "add", "path": "/dictlevelX1/dictlevelX2/dictlevelX3/dictlevelX4", "value": "nakamoto_secret"}] + jpatch = jsonpatch.JsonPatch(patches) + with self._customAssertRaises(JsonPointerException) as ctx: + data2 = jpatch.apply(data1) + fail_if_leaking_secret(ctx)