daemon: get_wallet: handle OSError for weiiird paths
What the developer wrote
Authored by SomberNight
daemon: get_wallet: handle OSError for weiiird paths
I think _wallet_key_from_path should not raise.
This is probably the sane way to deal with this.
Though all this is assuming that os.path.realpath can be treated as consistent/stateless.
closes https://github.com/spesmilo/electrum/issues/10182
What changed, and why it matters
This commit hardens Electrum's daemon so that unusual or malformed wallet file paths no longer cause the program to crash with an unhandled error. It wraps a filesystem path normalization call in a try/except block and logs a warning instead of raising an exception. The change is defensive and improves robustness, but it does not appear to fix an active security vulnerability.
Treat as a routine robustness improvement. No urgent security action required. Reviewers may want to confirm that falling back to the un-normalized path does not allow two different wallet keys to map to the same underlying file, which could reintroduce the duplicate-wallet-object risk noted in the adjacent comment.
Security signals we found
Defensive exception handling added to filesystem path normalization
Reference to 'weiiird paths' and issue #10182 suggests crash on unusual input
No cryptographic, authentication, or authorization changes
No memory safety, injection, or privilege escalation signals
Evidence from the diff
In electrum/daemon.py, _wallet_key_from_path now catches OSError from os.path.realpath(path, strict=False) and falls back to the original path, logging a warning. The strict=False parameter is new in Python 3.10 and avoids some symlink-related failures. A comment typo in load_recently_open_wallets was also fixed. electrum/gui/qml/qedaemon.py received a type-hint-only change. The referenced issue #10182 is not provided, so the exact failure mode is inferred from the code change.
Changed components
electrum/daemon.py:_wallet_key_from_pathelectrum/daemon.py:load_recently_open_walletselectrum/gui/qml/qedaemon.py:QEWalletListModel.__init__Inspect captured patch +7 / −3
diff --git a/electrum/daemon.py b/electrum/daemon.py
index 7b21b12..9114bab 100644
--- a/electrum/daemon.py
+++ b/electrum/daemon.py
@@ -462,7 +462,11 @@ class Daemon(Logger):
# note: the path returned by realpath has been observed NOT to work for FS operations!
# (e.g. for Cryptomator WinFSP/FUSE mounts, see #8495).
# It is okay for us to use it for computing a canonical wallet *key*, but cannot be used as a path!
- path = os.path.realpath(path)
+ try:
+ path = os.path.realpath(path, strict=False)
+ except OSError as e: # see #10182
+ _logger.warning(f"could not parse {path!r}: {e!r}")
+ path = path
# - "normcase" does Windows-specific case and slash normalisation:
path = os.path.normcase(path)
# - prepend header to break usage of wallet keys as fs paths
@@ -672,7 +676,7 @@ class Daemon(Logger):
if not os.path.isfile(path):
continue
wallet = self.get_wallet(path)
- # note: we only create a new wallet object if one was not loaded into the wallet already.
+ # note: we only create a new wallet object if one was not loaded into the daemon already.
# This is to avoid having two wallet objects contending for the same file.
# Take care: this only works if the daemon knows about all wallet objects.
# if other code already has created a Wallet() for a file but did not tell the daemon,
diff --git a/electrum/gui/qml/qedaemon.py b/electrum/gui/qml/qedaemon.py
index 651c93f..f9af213 100644
--- a/electrum/gui/qml/qedaemon.py
+++ b/electrum/gui/qml/qedaemon.py
@@ -38,7 +38,7 @@ class QEWalletListModel(QAbstractListModel):
_ROLE_KEYS = range(Qt.ItemDataRole.UserRole, Qt.ItemDataRole.UserRole + len(_ROLE_NAMES))
_ROLE_MAP = dict(zip(_ROLE_KEYS, [bytearray(x.encode()) for x in _ROLE_NAMES]))
- def __init__(self, daemon, parent=None):
+ def __init__(self, daemon: 'Daemon', parent=None):
QAbstractListModel.__init__(self, parent)
self.daemon = daemon
self._wallets = []
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.