retain the existing wallet encryption when a password change is cancelled

Sparrow/Sparrow Wallet

194bd70f View source ↗

Public commit record

What the developer wrote

Authored by Craig Raw

50/100 · Thin

retain the existing wallet encryption when a password change is cancelled

✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body

The short version

What changed, and why it matters

This commit fixes a bug in Sparrow Wallet where cancelling a password change could leave the wallet unencrypted in memory for the rest of the session. Previously, the software decrypted the wallet to prepare for a new password, but if the user cancelled, it never re-encrypted it. Now it re-encrypts with the existing key when the change is abandoned. This is a security fix for a local, user-interactive edge case.

Recommended action

Treat this as a security bug fix and include it in the next release. Users who previously cancelled password-change operations on an encrypted wallet should be advised to close and reopen Sparrow Wallet to clear any in-memory decrypted state, and to avoid leaving the application running on shared or untrusted systems.

Security signals we found

Wallet decryption state not restored on cancelled password change

Sensitive key material retained across UI thread handoff in terminal path

Password-derivation comparison logic changed to allow expected key change during password change

Re-encryption added for master wallet and child wallets when password change is abandoned

Technical analysis

Evidence from the diff

The patch changes saveWallet() in both the terminal and GUI settings controllers to return a boolean indicating whether a save was actually initiated. When the user is prompted to set/change a password (UPDATE_SET), the wallet is decrypted with the existing key and saveWallet(true, false) is called recursively. If that recursive call returns false (user cancelled or closed the dialog), the code now re-encrypts the master wallet and non-nested child wallets with the existing key and clears it, instead of leaving them decrypted. It also stops clearing the stored encryption public key before the password-change dialog, and adjusts the ‘incorrect password’ check so it does not fire during an actual password-change flow where the derived key is expected to differ.

Changed components

src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.javasrc/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java

Inspect captured patch +55 / −22

### src/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java
@@ -170,17 +170,16 @@ private void showSeed() {
         }
     }
 
-    private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
+    //Returns true if the wallet save was initiated, and false if it was abandoned without any change to the wallet or its storage
+    private boolean saveWallet(boolean changePassword, boolean suggestChangePassword) {
         WalletForm walletForm = getWalletForm();
         ECKey existingPubKey = walletForm.getStorage().getEncryptionPubKey();
 
         PasswordRequirement requirement;
-        if(existingPubKey == null) {
-            if(changePassword) {
-                requirement = PasswordRequirement.UPDATE_CHANGE;
-            } else {
-                requirement = PasswordRequirement.UPDATE_NEW;
-            }
+        if(changePassword) {
+            requirement = PasswordRequirement.UPDATE_CHANGE;
+        } else if(existingPubKey == null) {
+            requirement = PasswordRequirement.UPDATE_NEW;
         } else if(Storage.NO_PASSWORD_KEY.equals(existingPubKey)) {
             requirement = PasswordRequirement.UPDATE_EMPTY;
         } else {
@@ -213,7 +212,8 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                         try {
                             ECKey encryptionPubKey = ECKey.fromPublicOnly(encryptionFullKey);
 
-                            if(existingPubKey != null && !Storage.NO_PASSWORD_KEY.equals(existingPubKey) && !existingPubKey.equals(encryptionPubKey)) {
+                            //When changing the password, the existing encryption key is retained until the new one is derived, so a different key is expected here
+                            if(!changePassword && existingPubKey != null && !Storage.NO_PASSWORD_KEY.equals(existingPubKey) && !existingPubKey.equals(encryptionPubKey)) {
                                 AppServices.showErrorDialog("Incorrect Password", "The password was incorrect.");
                                 return;
                             }
@@ -222,14 +222,32 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
 
                             Wallet masterWallet = walletForm.getWallet().isMasterWallet() ? walletForm.getWallet() : walletForm.getWallet().getMasterWallet();
                             if(suggestChangePassword && requirement == PasswordRequirement.UPDATE_SET) {
-                                walletForm.getStorage().setEncryptionPubKey(null);
                                 masterWallet.decrypt(key);
                                 for(Wallet childWallet : masterWallet.getChildWallets()) {
                                     if(!childWallet.isNested()) {
                                         childWallet.decrypt(key);
                                     }
                                 }
-                                SparrowTerminal.get().getGuiThread().invokeLater(() -> saveWallet(true, false));
+
+                                //The next dialog is shown on the gui thread, so hand the existing key over to re-encrypt with rather than clearing it here
+                                Key existingKey = key;
+                                key = null;
+                                SparrowTerminal.get().getGuiThread().invokeLater(() -> {
+                                    boolean saving = saveWallet(true, false);
+                                    Platform.runLater(() -> {
+                                        //If a new password is not provided, re-encrypt with the existing key rather than leaving the wallet decrypted for the session
+                                        if(!saving) {
+                                            masterWallet.encrypt(existingKey);
+                                            for(Wallet childWallet : masterWallet.getChildWallets()) {
+                                                if(!childWallet.isNested()) {
+                                                    childWallet.encrypt(existingKey);
+                                                }
+                                            }
+                                        }
+
+                                        existingKey.clear();
+                                    });
+                                });
                                 return;
                             }
 
@@ -259,7 +277,11 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                     keyDerivationService.start();
                 }
             });
+
+            return true;
         }
+
+        return false;
     }
 
     public static List<String> splitString(String stringToSplit, int maxLength) {

### src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
@@ -956,16 +956,15 @@ public void existingWalletImported(ExistingWalletImportedEvent event) {
         }
     }
 
-    private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
+    //Returns true if the wallet save was initiated, and false if it was abandoned without any change to the wallet or its storage
+    private boolean saveWallet(boolean changePassword, boolean suggestChangePassword) {
         ECKey existingPubKey = walletForm.getStorage().getEncryptionPubKey();
 
         WalletPasswordDialog.PasswordRequirement requirement;
-        if(existingPubKey == null) {
-            if(changePassword) {
-                requirement = WalletPasswordDialog.PasswordRequirement.UPDATE_CHANGE;
-            } else {
-                requirement = WalletPasswordDialog.PasswordRequirement.UPDATE_NEW;
-            }
+        if(changePassword) {
+            requirement = WalletPasswordDialog.PasswordRequirement.UPDATE_CHANGE;
+        } else if(existingPubKey == null) {
+            requirement = WalletPasswordDialog.PasswordRequirement.UPDATE_NEW;
         } else if(Storage.NO_PASSWORD_KEY.equals(existingPubKey)) {
             requirement = WalletPasswordDialog.PasswordRequirement.UPDATE_EMPTY;
         } else {
@@ -977,7 +976,7 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
             if(optResponse.isPresent() && optResponse.get().equals(ButtonType.CANCEL)) {
                 revert.setDisable(false);
                 apply.setDisable(false);
-                return;
+                return false;
             }
         }
 
@@ -993,7 +992,7 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                     AppServices.showErrorDialog("Error saving wallet backup", e.getMessage());
                     revert.setDisable(false);
                     apply.setDisable(false);
-                    return;
+                    return false;
                 }
             }
 
@@ -1018,7 +1017,8 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                     try {
                         ECKey encryptionPubKey = ECKey.fromPublicOnly(encryptionFullKey);
 
-                        if(existingPubKey != null && !Storage.NO_PASSWORD_KEY.equals(existingPubKey) && !existingPubKey.equals(encryptionPubKey)) {
+                        //When changing the password, the existing encryption key is retained until the new one is derived, so a different key is expected here
+                        if(!changePassword && existingPubKey != null && !Storage.NO_PASSWORD_KEY.equals(existingPubKey) && !existingPubKey.equals(encryptionPubKey)) {
                             AppServices.showErrorDialog("Incorrect Password", "The password was incorrect.");
                             revert.setDisable(false);
                             apply.setDisable(false);
@@ -1033,14 +1033,22 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                                 walletForm.deleteBackups();
                             }
 
-                            walletForm.getStorage().setEncryptionPubKey(null);
                             masterWallet.decrypt(key);
                             for(Wallet childWallet : masterWallet.getChildWallets()) {
                                 if(!childWallet.isNested()) {
                                     childWallet.decrypt(key);
                                 }
                             }
-                            saveWallet(true, false);
+
+                            //If a new password is not provided, re-encrypt with the existing key rather than leaving the wallet decrypted for the session
+                            if(!saveWallet(true, false)) {
+                                masterWallet.encrypt(key);
+                                for(Wallet childWallet : masterWallet.getChildWallets()) {
+                                    if(!childWallet.isNested()) {
+                                        childWallet.encrypt(key);
+                                    }
+                                }
+                            }
                             return;
                         }
 
@@ -1077,9 +1085,12 @@ private void saveWallet(boolean changePassword, boolean suggestChangePassword) {
                 EventManager.get().post(new StorageEvent(walletForm.getWalletId(), TimedEvent.Action.START, "Encrypting wallet..."));
                 keyDerivationService.start();
             }
+
+            return true;
         } else {
             revert.setDisable(false);
             apply.setDisable(false);
+            return false;
         }
     }

Risk score

Why this scored 59/100

Our methodology →

Potential impact 18/30

Exploitability 12/25

Stealth signal 10/15

Affected reach 8/15

Confidence 7/10

Evidence quality 4/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 59/100

Community notes

Related watches

remove the surplus signatures progress bar segments a finalized multisig transaction discards

bump to v2.5.6

use a framerate-capped interpolated timeline for the server toggle and wallet tab loading pulse animations, and stop any running server toggle pulse before starting a new one