JsonDB: monkeypatch jsonpatch exceptions to avoid leaking secrets

closes https://github.com/spesmilo/electrum/issues/10001

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: """<JsonPointerException 'redacted'>"""
+jsonpatch.JsonPatchException.__str__ = lambda self: """(JPE) 'redacted'"""
+jsonpatch.JsonPatchException.__repr__ = lambda self: """<JsonPatchException 'redacted'>"""
+
+
 def modifier(func):
     def wrapper(self, *args, **kwargs):
         with self.lock:

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)