qml: enforce use of existing password for wallet creation
What changed, and why it matters
This commit is a hardening and usability change for Electrum's mobile/QML wallet creation wizard. When the app is configured to prefer a single password across wallets, it now forces the user to reuse an existing wallet password when creating a new wallet, instead of allowing a new password. It also renames an internal configuration flag and exposes helper functions to check how many wallets a password can unlock. There is no direct security vulnerability being patched; it is a defensive measure to reduce password fragmentation.
No immediate action required. Reviewers may want to confirm that the password comparison timer does not leak timing information and that check_password_for_directory cannot be invoked in a way that allows unauthenticated password guessing beyond the existing threat model. Consider whether rate limiting or logging is appropriate for repeated calls to numWalletsWithPassword.
Security signals we found
Renames configuration flag from WALLET_USE_SINGLE_PASSWORD to WALLET_SHOULD_USE_SINGLE_PASSWORD
Makes password-directory check public and returns unlockable wallet paths
Adds QML bindings to count wallets unlockable with a candidate password
Enforces password reuse during new-wallet creation in QML wizard when single-password mode is enabled
Adds user-facing guidance text about password unification
Evidence from the diff
The patch renames WALLET_USE_SINGLE_PASSWORD to WALLET_SHOULD_USE_SINGLE_PASSWORD and changes _check_password_for_directory to a public check_password_for_directory that additionally returns the list of wallet paths that could be unlocked. It adds QML-exposed methods getWalletsUnlockableWithPassword and numWalletsWithPassword, and updates the WCWalletPassword wizard page so that when Config.walletShouldUseSinglePassword is true and other wallets exist, the user must enter a password matching an existing wallet. The password is checked against all wallets in the directory via check_password_for_directory. Tests are updated to match the new return signature.
Changed components
electrum/daemon.pyelectrum/gui/qml/components/wizard/WCWalletPassword.qmlelectrum/gui/qml/qeconfig.pyelectrum/gui/qml/qedaemon.pyelectrum/simple_config.pyrun_electrumtests/test_daemon.pyInspect captured patch +119 / −23
diff --git a/electrum/daemon.py b/electrum/daemon.py
index 3afa92a..b1ab1e8 100644
--- a/electrum/daemon.py
+++ b/electrum/daemon.py
@@ -663,11 +663,12 @@ class Daemon(Logger):
asyncio.run_coroutine_threadsafe(self.stop(), self.asyncio_loop).result()
@with_wallet_lock
- def _check_password_for_directory(self, *, old_password, new_password=None, wallet_dir: str) -> Tuple[bool, bool]:
+ def check_password_for_directory(self, *, old_password, new_password=None, wallet_dir: str) -> Tuple[bool, bool, list[str]]:
"""Checks password against all wallets (in dir), returns whether they can be unified and whether they are already.
If new_password is not None, update all wallet passwords to new_password.
"""
assert os.path.exists(wallet_dir), f"path {wallet_dir!r} does not exist"
+ succeeded = []
failed = []
is_unified = True
for filename in os.listdir(wallet_dir):
@@ -706,9 +707,11 @@ class Daemon(Logger):
if new_password:
self.logger.info(f'updating password for wallet: {path!r}')
wallet.update_password(old_password_real, new_password, encrypt_storage=True)
+ succeeded.append(path)
+
can_be_unified = failed == []
is_unified = can_be_unified and is_unified
- return can_be_unified, is_unified
+ return can_be_unified, is_unified, succeeded
@with_wallet_lock
def update_password_for_directory(
@@ -724,13 +727,13 @@ class Daemon(Logger):
return False
if wallet_dir is None:
wallet_dir = os.path.dirname(self.config.get_wallet_path())
- can_be_unified, is_unified = self._check_password_for_directory(
+ can_be_unified, is_unified, _ = self.check_password_for_directory(
old_password=old_password, new_password=None, wallet_dir=wallet_dir)
if not can_be_unified:
return False
if is_unified and old_password == new_password:
return True
- self._check_password_for_directory(
+ self.check_password_for_directory(
old_password=old_password, new_password=new_password, wallet_dir=wallet_dir)
return True
diff --git a/electrum/gui/qml/components/wizard/WCWalletPassword.qml b/electrum/gui/qml/components/wizard/WCWalletPassword.qml
index eafd6e7..b71bfef 100644
--- a/electrum/gui/qml/components/wizard/WCWalletPassword.qml
+++ b/electrum/gui/qml/components/wizard/WCWalletPassword.qml
@@ -5,37 +5,70 @@ import QtQuick.Controls.Material
import "../controls"
+// We will only end up here if Daemon.singlePasswordEnabled == False.
+// If there are existing wallets, the user must reuse the password of one of them.
+// This way they are guided towards password unification.
+// NOTE: This also needs to be enforced when changing a wallets password.
+
WizardComponent {
- valid: password1.text === password2.text && password1.text.length >= 6
+ id: root
+ valid: isInputValid()
+ property bool enforceExistingPassword: Config.walletShouldUseSinglePassword && Daemon.availableWallets.rowCount() > 0
+ property bool passwordMatchesAnyExisting: false
function apply() {
wizard_data['password'] = password1.text
wizard_data['encrypt'] = password1.text != ''
}
+ function isInputValid() {
+ if (password1.text == "") {
+ return false
+ }
+ if (enforceExistingPassword) {
+ return passwordMatchesAnyExisting
+ }
+ return password1.text === password2.text && password1.text.length >= 6
+ }
+
+ Timer {
+ id: passwordComparisonTimer
+ interval: 500
+ repeat: false
+ onTriggered: {
+ root.passwordMatchesAnyExisting = Daemon.numWalletsWithPassword(password1.text) > 0
+ }
+ }
+
ColumnLayout {
anchors.fill: parent
Label {
Layout.fillWidth: true
- text: Daemon.singlePasswordEnabled
- ? qsTr('Enter password')
- : qsTr('Enter password for %1').arg(wizard_data['wallet_name'])
+ text: !enforceExistingPassword ? qsTr('Enter password') : qsTr('Enter existing password')
wrapMode: Text.Wrap
}
PasswordField {
id: password1
+ onTextChanged: {
+ if (enforceExistingPassword) {
+ root.passwordMatchesAnyExisting = false
+ passwordComparisonTimer.restart()
+ }
+ }
}
Label {
text: qsTr('Enter password (again)')
+ visible: !enforceExistingPassword
}
PasswordField {
id: password2
showReveal: false
echoMode: password1.echoMode
+ visible: !enforceExistingPassword
}
RowLayout {
@@ -44,7 +77,7 @@ WizardComponent {
Layout.rightMargin: constants.paddingXLarge
Layout.topMargin: constants.paddingXLarge
- visible: password1.text != ''
+ visible: password1.text != '' && !enforceExistingPassword
Label {
Layout.rightMargin: constants.paddingLarge
@@ -65,13 +98,30 @@ WizardComponent {
InfoTextArea {
Layout.alignment: Qt.AlignCenter
text: qsTr('Passwords don\'t match')
- visible: password1.text != password2.text
+ visible: (password1.text != password2.text) && !enforceExistingPassword
iconStyle: InfoTextArea.IconStyle.Warn
}
InfoTextArea {
Layout.alignment: Qt.AlignCenter
text: qsTr('Password too short')
- visible: (password1.text == password2.text) && !valid
+ visible: (password1.text == password2.text) && !valid && !enforceExistingPassword
+ iconStyle: InfoTextArea.IconStyle.Warn
+ }
+ InfoTextArea {
+ Layout.alignment: Qt.AlignCenter
+ Layout.fillWidth: true
+ visible: password1.text == "" && enforceExistingPassword
+ text: [
+ qsTr("Use the password of any existing wallet."),
+ qsTr("Creating new wallets with different passwords is not supported.")
+ ].join("\n")
+ iconStyle: InfoTextArea.IconStyle.Info
+ }
+ InfoTextArea {
+ Layout.alignment: Qt.AlignCenter
+ Layout.fillWidth: true
+ visible: password1.text != "" && !valid && enforceExistingPassword
+ text: qsTr('Password does not match any existing wallets password.')
iconStyle: InfoTextArea.IconStyle.Warn
}
}
diff --git a/electrum/gui/qml/qeconfig.py b/electrum/gui/qml/qeconfig.py
index 38cd806..9bf2a29 100644
--- a/electrum/gui/qml/qeconfig.py
+++ b/electrum/gui/qml/qeconfig.py
@@ -331,6 +331,15 @@ class QEConfig(AuthMixin, QObject):
self._lnutxoreserve = QEAmount(amount_sat=self.config.LN_UTXO_RESERVE)
return self._lnutxoreserve
+ walletShouldUseSinglePasswordChanged = pyqtSignal()
+ @pyqtProperty(bool, notify=walletShouldUseSinglePasswordChanged)
+ def walletShouldUseSinglePassword(self):
+ """
+ NOTE: this only indicates if we even want to use a single password, to check if we
+ actually use a single password the daemon needs to be checked.
+ """
+ return self.config.WALLET_SHOULD_USE_SINGLE_PASSWORD
+
@pyqtSlot('qint64', result=str)
@pyqtSlot(QEAmount, result=str)
def formatSatsForEditing(self, satoshis):
diff --git a/electrum/gui/qml/qedaemon.py b/electrum/gui/qml/qedaemon.py
index f9af213..c31937d 100644
--- a/electrum/gui/qml/qedaemon.py
+++ b/electrum/gui/qml/qedaemon.py
@@ -230,7 +230,7 @@ class QEDaemon(AuthMixin, QObject):
if wallet is None:
return
- if self.daemon.config.WALLET_USE_SINGLE_PASSWORD:
+ if self.daemon.config.WALLET_SHOULD_USE_SINGLE_PASSWORD:
self._use_single_password = self.daemon.update_password_for_directory(old_password=local_password, new_password=local_password)
self._password = local_password
self.singlePasswordChanged.emit()
@@ -318,9 +318,40 @@ class QEDaemon(AuthMixin, QObject):
def fx(self):
return self.qefx
+ @pyqtSlot(str, result=list)
+ def getWalletsUnlockableWithPassword(self, password: str) -> list[str]:
+ """
+ Returns any wallet that can be unlocked with the given password.
+ Can be used as fallback to unlock another wallet the user entered a
+ password that doesn't work for the current wallet but might work for another one.
+ """
+ wallet_dir = os.path.dirname(self.daemon.config.get_wallet_path())
+ _, _, wallet_paths_can_unlock = self.daemon.check_password_for_directory(
+ old_password=password,
+ new_password=None,
+ wallet_dir=wallet_dir,
+ )
+ if not wallet_paths_can_unlock:
+ return []
+ self._logger.debug(f"getWalletsUnlockableWithPassword: can unlock {len(wallet_paths_can_unlock)} wallets")
+ return [str(path) for path in wallet_paths_can_unlock]
+
+ @pyqtSlot(str, result=int)
+ def numWalletsWithPassword(self, password: str) -> int:
+ """Returns the number of wallets that can be unlocked with the given password"""
+ wallet_paths_can_unlock = self.getWalletsUnlockableWithPassword(password)
+ return len(wallet_paths_can_unlock)
+
singlePasswordChanged = pyqtSignal()
@pyqtProperty(bool, notify=singlePasswordChanged)
def singlePasswordEnabled(self):
+ """
+ singlePasswordEnabled is False if:
+ a.) the user has no wallet (and password) yet
+ b.) the user has wallets with different passwords (legacy)
+ c.) all wallets are locked, we couldn't check yet if they all use the same password
+ d.) we are on desktop where different passwords are allowed
+ """
return self._use_single_password
@pyqtProperty(str, notify=singlePasswordChanged)
diff --git a/electrum/simple_config.py b/electrum/simple_config.py
index 32638bd..92b11de 100644
--- a/electrum/simple_config.py
+++ b/electrum/simple_config.py
@@ -675,7 +675,7 @@ class SimpleConfig(Logger):
)
WALLET_UNCONF_UTXO_FREEZE_THRESHOLD_SAT = ConfigVar('unconf_utxo_freeze_threshold', default=5_000, type_=int)
WALLET_PAYREQ_EXPIRY_SECONDS = ConfigVar('request_expiry', default=invoices.PR_DEFAULT_EXPIRATION_WHEN_CREATING, type_=int)
- WALLET_USE_SINGLE_PASSWORD = ConfigVar('single_password', default=False, type_=bool)
+ WALLET_SHOULD_USE_SINGLE_PASSWORD = ConfigVar('should_use_single_password', default=False, type_=bool)
# note: 'use_change' and 'multiple_change' are per-wallet settings
WALLET_SEND_CHANGE_TO_LIGHTNING = ConfigVar(
'send_change_to_lightning', default=False, type_=bool,
diff --git a/run_electrum b/run_electrum
index 3d75478..df45836 100755
--- a/run_electrum
+++ b/run_electrum
@@ -397,7 +397,7 @@ def main():
'verbosity': '*' if util.is_android_debug_apk() else '',
'cmd': 'gui',
SimpleConfig.GUI_NAME.key(): 'qml',
- SimpleConfig.WALLET_USE_SINGLE_PASSWORD.key(): True,
+ SimpleConfig.WALLET_SHOULD_USE_SINGLE_PASSWORD.key(): True,
}
SimpleConfig.set_chain_config_opt_based_on_android_packagename(config_options)
else:
diff --git a/tests/test_daemon.py b/tests/test_daemon.py
index 08f53cf..f5bbbad 100644
--- a/tests/test_daemon.py
+++ b/tests/test_daemon.py
@@ -52,7 +52,7 @@ class TestUnifiedPassword(DaemonTestCase):
def setUp(self):
super().setUp()
- self.config.WALLET_USE_SINGLE_PASSWORD = True
+ self.config.WALLET_SHOULD_USE_SINGLE_PASSWORD = True
def _run_post_unif_sanity_checks(self, paths: Iterable[str], *, password: str):
for path in paths:
@@ -64,8 +64,11 @@ class TestUnifiedPassword(DaemonTestCase):
self.assertTrue(w.has_keystore_encryption())
if w.has_seed():
self.assertIsInstance(w.get_seed(password), str)
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password=password, wallet_dir=self.wallet_dir)
- self.assertEqual((True, True), (can_be_unified, is_unified))
+ can_be_unified, is_unified, wallet_paths_can_unlock = self.daemon.check_password_for_directory(
+ old_password=password,
+ wallet_dir=self.wallet_dir,
+ )
+ self.assertEqual((True, True, len(paths)), (can_be_unified, is_unified, len(wallet_paths_can_unlock)))
# "cannot unify pw" tests --->
@@ -77,7 +80,7 @@ class TestUnifiedPassword(DaemonTestCase):
with open(path2, "rb") as f:
raw2_before = f.read()
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((False, False), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertFalse(is_unified)
@@ -100,7 +103,7 @@ class TestUnifiedPassword(DaemonTestCase):
with open(path3, "rb") as f:
raw3_before = f.read()
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((False, False), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertFalse(is_unified)
@@ -120,7 +123,7 @@ class TestUnifiedPassword(DaemonTestCase):
async def test_can_unify_two_std_wallets_both_have_ks_and_sto_enc(self):
path1 = self._restore_wallet_from_text("9dk", password="123456", encrypt_file=True)
path2 = self._restore_wallet_from_text("x8", password="123456", encrypt_file=True)
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((True, True), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertTrue(is_unified)
@@ -132,7 +135,7 @@ class TestUnifiedPassword(DaemonTestCase):
with open(path2, "rb") as f:
raw2_before = f.read()
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((True, False), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertTrue(is_unified)
@@ -145,7 +148,7 @@ class TestUnifiedPassword(DaemonTestCase):
async def test_can_unify_two_std_wallets_one_without_password(self):
path1 = self._restore_wallet_from_text("9dk", password=None)
path2 = self._restore_wallet_from_text("x8", password="123456", encrypt_file=True)
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((True, False), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertTrue(is_unified)
@@ -179,7 +182,7 @@ class TestUnifiedPassword(DaemonTestCase):
paths.append(self._restore_wallet_from_text(addrs, password="123456", encrypt_file=False))
paths.append(self._restore_wallet_from_text(addrs, password=None))
# do unification
- can_be_unified, is_unified = self.daemon._check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
+ can_be_unified, is_unified, _ = self.daemon.check_password_for_directory(old_password="123456", wallet_dir=self.wallet_dir)
self.assertEqual((True, False), (can_be_unified, is_unified))
is_unified = self.daemon.update_password_for_directory(old_password="123456", new_password="123456")
self.assertTrue(is_unified)
Why this scored 19/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.