What the developer wrote
Authored by f321x
qml: deduplicate wallet name validation
Deduplicates the wallet name validation between wizard and daemon.
What changed, and why it matters
This commit cleans up duplicate wallet-name checking code in Electrum's mobile-style QML interface and moves the validation into one shared place. It also slightly changes when the 'rename' button is enabled. The changes look like a defensive hardening/refactoring rather than a fix for a known active attack, but they do strengthen checks against path-traversal-style wallet names.
Treat as a low-risk hardening/refactoring commit. Review whether removing the 'name must differ' guard in WalletDetails.qml could allow a no-op rename request to reach the daemon, and confirm the daemon handles identical-name renames gracefully. No urgent action required.
Security signals we found
Path-traversal prevention in wallet name validation (forbidden '/' and '\', basename check)
Filesystem-level validation using WalletStorage before accepting a wallet name
Removal of duplicate validation logic reduces risk of inconsistent checks
Rename button no longer requires name to differ from current wallet; relies on validator
Evidence from the diff
The patch deduplicates wallet name validation between QENewWalletWizard and QEDaemon by removing isValidNewWalletName from qewizard.py and adding/reusing isValidWalletName in qedaemon.py. WCWalletName.qml now calls Daemon.isValidWalletName instead of wiz.isValidNewWalletName. WalletDetails.qml drops the explicit ‘name must differ from current wallet’ check and relies solely on Daemon.isValidWalletName to enable the rename button. The validation logic itself is preserved: empty names, existing names, slashes/backslashes, basename mismatch, and WalletStorage sanity/file-existence checks all still reject invalid names. The path helper wallet_path_from_wallet_name was moved from wizard to daemon.
Changed components
electrum/gui/qml/qedaemon.pyelectrum/gui/qml/qewizard.pyelectrum/gui/qml/components/wizard/WCWalletName.qmlelectrum/gui/qml/components/WalletDetails.qmlInspect captured patch +24 / −37
diff --git a/electrum/gui/qml/components/WalletDetails.qml b/electrum/gui/qml/components/WalletDetails.qml
index 2676c58..9db036f 100644
--- a/electrum/gui/qml/components/WalletDetails.qml
+++ b/electrum/gui/qml/components/WalletDetails.qml
@@ -172,8 +172,7 @@ Pane {
visible: walletNameContent.editmode
icon.source: '../../icons/confirmed.png'
icon.color: enabled ? 'transparent' : constants.mutedForeground
- enabled: walletNameEdit.text !== Daemon.currentWallet.name
- && Daemon.isValidWalletName(walletNameEdit.text)
+ enabled: Daemon.isValidWalletName(walletNameEdit.text)
onClicked: {
walletNameContent.editmode = false
Daemon.renameWallet(walletNameEdit.text)
diff --git a/electrum/gui/qml/components/wizard/WCWalletName.qml b/electrum/gui/qml/components/wizard/WCWalletName.qml
index 7dc243d..1020b03 100644
--- a/electrum/gui/qml/components/wizard/WCWalletName.qml
+++ b/electrum/gui/qml/components/wizard/WCWalletName.qml
@@ -5,7 +5,7 @@ import QtQuick.Controls
import org.electrum 1.0
WizardComponent {
- valid: wiz.isValidNewWalletName(wallet_name.text)
+ valid: Daemon.isValidWalletName(wallet_name.text)
function apply() {
wizard_data['wallet_name'] = wallet_name.text
diff --git a/electrum/gui/qml/qedaemon.py b/electrum/gui/qml/qedaemon.py
index d9eadb3..b8509bd 100644
--- a/electrum/gui/qml/qedaemon.py
+++ b/electrum/gui/qml/qedaemon.py
@@ -13,7 +13,7 @@ from electrum.plugin import run_hook
from electrum.lnchannel import ChannelState
from electrum.bitcoin import is_address
from electrum.bitcoin import verify_usermessage_with_address
-from electrum.storage import StorageReadWriteError
+from electrum.storage import StorageReadWriteError, WalletStorage
from .auth import AuthMixin, auth_protect
from .qefx import QEFX
@@ -311,15 +311,33 @@ class QEDaemon(AuthMixin, QObject):
self.availableWallets.remove_wallet(path)
+ def wallet_path_from_wallet_name(self, wallet_name: str) -> str:
+ return os.path.join(self.daemon.config.get_datadir_wallet_path(), wallet_name)
+
@pyqtSlot(str, result=bool)
def isValidWalletName(self, wallet_name: str) -> bool:
if not wallet_name:
return False
if self.availableWallets.wallet_name_exists(wallet_name):
return False
- # ensure wallet_name is not interpreted as path
+ # note: we should probably restrict wallet names to be alphanumeric (plus underscore, etc)...
+ # try to prevent sketchy path traversals:
+ for forbidden_char in ("/", "\\", ):
+ if forbidden_char in wallet_name:
+ return False
if os.path.basename(wallet_name) != wallet_name: # '/foo/bar/' returns 'bar'
return False
+ wallet_path = self.wallet_path_from_wallet_name(wallet_name)
+ # validate that the path looks sane to the filesystem:
+ try:
+ temp_storage = WalletStorage(wallet_path)
+ except (StorageReadWriteError, WalletFileException):
+ return False
+ except Exception:
+ self._logger.exception("")
+ return False
+ if temp_storage.file_exists():
+ return False
return True
@pyqtSlot(str)
diff --git a/electrum/gui/qml/qewizard.py b/electrum/gui/qml/qewizard.py
index ca1961f..3bd3f33 100644
--- a/electrum/gui/qml/qewizard.py
+++ b/electrum/gui/qml/qewizard.py
@@ -7,8 +7,7 @@ from electrum.base_crash_reporter import send_exception_to_crash_reporter
from electrum.logging import get_logger
from electrum import mnemonic
from electrum.wizard import NewWalletWizard, ServerConnectWizard, TermsOfUseWizard
-from electrum.storage import WalletStorage, StorageReadWriteError
-from electrum.util import WalletFileException, UserFacingException
+from electrum.util import UserFacingException
from electrum.gui import messages
if TYPE_CHECKING:
@@ -125,35 +124,6 @@ class QENewWalletWizard(NewWalletWizard, QEAbstractWizard):
'can_passphrase': can_passphrase
}
- def _wallet_path_from_wallet_name(self, wallet_name: str) -> str:
- return os.path.join(self._qedaemon.daemon.config.get_datadir_wallet_path(), wallet_name)
-
- @pyqtSlot(str, result=bool)
- def isValidNewWalletName(self, wallet_name: str) -> bool:
- if not wallet_name:
- return False
- if self._qedaemon.availableWallets.wallet_name_exists(wallet_name):
- return False
- wallet_path = self._wallet_path_from_wallet_name(wallet_name)
- # note: we should probably restrict wallet names to be alphanumeric (plus underscore, etc)...
- # try to prevent sketchy path traversals:
- for forbidden_char in ("/", "\\", ):
- if forbidden_char in wallet_name:
- return False
- if os.path.basename(wallet_name) != wallet_name:
- return False
- # validate that the path looks sane to the filesystem:
- try:
- temp_storage = WalletStorage(wallet_path)
- except (StorageReadWriteError, WalletFileException) as e:
- return False
- except Exception as e:
- self._logger.exception("")
- return False
- if temp_storage.file_exists():
- return False
- return True
-
@pyqtSlot('QJSValue', bool, str)
def createStorage(self, js_data, single_password_enabled, single_password):
self._logger.info('Creating wallet from wizard data')
@@ -163,7 +133,7 @@ class QENewWalletWizard(NewWalletWizard, QEAbstractWizard):
data['encrypt'] = True
data['password'] = single_password
- path = self._wallet_path_from_wallet_name(data['wallet_name'])
+ path = self._qedaemon.wallet_path_from_wallet_name(data['wallet_name'])
try:
self.create_storage(path, data)
Why this scored 29/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.