remove ineffective eckey clear

Sparrow/Sparrow Wallet

1636c7e2 View source ↗

Public commit record

What the developer wrote

Authored by Craig Raw

35/100 · Opaque

remove ineffective eckey clear

✓ Descriptive subject! No meaningful explanatory body

The short version

What changed, and why it matters

This commit removes several calls that tried to wipe private-key material from memory. The developer says these wipes were 'ineffective' because the underlying ECKey object still held the secret bytes. The change is a cleanup, but it slightly increases the window during which private key bytes may sit in memory. It does not introduce a remotely exploitable bug; the main risk is a small worsening of local memory-exposure hygiene.

Recommended action

Treat as a minor defensive cleanup rather than a critical vulnerability. Review whether ECKey or its subclass should implement effective secure clearing, and ensure that derived Key objects and SecureString passwords are still being cleared promptly. No urgent patch or advisory is warranted from this commit alone.

Security signals we found

Removal of explicit private-key zeroization calls

Commit message labels prior zeroization as ineffective

Private key material remains referenced longer after use

One location changes clear target from masterPrivateKey to masterPrivateExtendedKey

Technical analysis

Evidence from the diff

The patch deletes .clear() invocations on ECKey instances (encryptionFullKey, spendPrivKey, privKey) across the GUI, terminal, and storage code. It also removes an encryptionKey.clear() in WalletAndKey and changes Storage.java to call clear() on the master private extended key object instead of the master private key object. The commit message states these ECKey clears were ineffective. The practical effect is that previously the code attempted to zero out key material but failed because ECKey.clear() did not actually clear the private bytes; removing the calls makes the no-op explicit. One change in Storage.java may actually improve clearing by targeting the correct extended-key object.

Changed components

Wallet import/save flow (AppController, NewWalletDialog)Wallet delete/unlock flow (AppController, MasterActionListBox, WalletDialog)Message signing (MessageSignDialog)Storage key derivation (Storage, WalletAndKey)Settings/seed display (SettingsController, SettingsDialog)

Inspect captured patch +2 / −19

diff --git a/src/main/java/com/sparrowwallet/sparrow/AppController.java b/src/main/java/com/sparrowwallet/sparrow/AppController.java
index 4ff89e4..2ebcedf 100644
--- a/src/main/java/com/sparrowwallet/sparrow/AppController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/AppController.java
@@ -1431,7 +1431,6 @@ public class AppController implements Initializable {
                     } catch(IOException | StorageException | MnemonicException e) {
                         log.error("Error saving imported wallet", e);
                     } finally {
-                        encryptionFullKey.clear();
                         if(key != null) {
                             key.clear();
                         }
@@ -2423,13 +2422,11 @@ public class AppController implements Initializable {
                     keyDerivationService = new Storage.KeyDerivationService(storage, password.get(), true);
                     keyDerivationService.setOnSucceeded(workerStateEvent -> {
                         EventManager.get().post(new StorageEvent(selectedWalletForm.getWalletId(), TimedEvent.Action.END, "Done"));
-                        ECKey encryptionFullKey = keyDerivationService.getValue();
 
                         try {
                             tabs.getTabs().remove(tabs.getSelectionModel().getSelectedItem());
                             deleteStorage(storage, true);
                         } finally {
-                            encryptionFullKey.clear();
                             keyDerivationService = null;
                         }
                     });
diff --git a/src/main/java/com/sparrowwallet/sparrow/AppServices.java b/src/main/java/com/sparrowwallet/sparrow/AppServices.java
index 71d078f..aa83a4a 100644
--- a/src/main/java/com/sparrowwallet/sparrow/AppServices.java
+++ b/src/main/java/com/sparrowwallet/sparrow/AppServices.java
@@ -1146,7 +1146,6 @@ public class AppServices {
                                 showErrorDialog("Error authenticating", "Failed to authenticate.\n\n" + e.getMessage());
                             } finally {
                                 key.clear();
-                                encryptionFullKey.clear();
                                 password.get().clear();
                             }
                         });
diff --git a/src/main/java/com/sparrowwallet/sparrow/control/MessageSignDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/MessageSignDialog.java
index 42e0f3d..4732f9f 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/MessageSignDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/MessageSignDialog.java
@@ -392,7 +392,6 @@ public class MessageSignDialog extends Dialog<ButtonBar.ButtonData> {
             if(decryptedWallet.getPolicyType() == PolicyType.SINGLE_SP) {
                 ECKey spendPrivKey = keystore.getSpendPrivateKey(Collections.emptyMap());
                 signatureText = Bip322.signMessageBip322Sp(walletNode.getAddress(), message.getText().trim(), spendPrivKey, walletNode.getSilentPaymentTweak());
-                spendPrivKey.clear();
             } else {
                 ECKey privKey = keystore.getKey(walletNode);
                 if(isBip322()) {
@@ -402,7 +401,6 @@ public class MessageSignDialog extends Dialog<ButtonBar.ButtonData> {
                     ScriptType scriptType = isElectrumSignatureFormat() ? ScriptType.P2PKH : decryptedWallet.getScriptType();
                     signatureText = privKey.signMessage(message.getText().trim(), scriptType);
                 }
-                privKey.clear();
             }
             signature.clear();
             signature.appendText(signatureText);
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
index ea93357..e5ee3f1 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
@@ -201,7 +201,7 @@ public class Storage {
                     keystore.setExtendedPublicKey(derivedKeystore.getExtendedPublicKey());
                     keystore.setBip47ExtendedPrivateKey(derivedKeystore.getBip47ExtendedPrivateKey());
                     keystore.setSilentPaymentScanAddress(derivedKeystore.getSilentPaymentScanAddress());
-                    copyKeystore.getMasterPrivateKey().clear();
+                    copyKeystore.getMasterPrivateExtendedKey().clear();
                 }
             }
         }
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/WalletAndKey.java b/src/main/java/com/sparrowwallet/sparrow/io/WalletAndKey.java
index b18d6e6..6768eaa 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/WalletAndKey.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/WalletAndKey.java
@@ -35,9 +35,6 @@ public class WalletAndKey implements Comparable<WalletAndKey> {
     }
 
     public void clear() {
-        if(encryptionKey != null) {
-            encryptionKey.clear();
-        }
         if(key != null) {
             key.clear();
         }
diff --git a/src/main/java/com/sparrowwallet/sparrow/terminal/MasterActionListBox.java b/src/main/java/com/sparrowwallet/sparrow/terminal/MasterActionListBox.java
index 0ee1b9d..5a9c7ca 100644
--- a/src/main/java/com/sparrowwallet/sparrow/terminal/MasterActionListBox.java
+++ b/src/main/java/com/sparrowwallet/sparrow/terminal/MasterActionListBox.java
@@ -108,7 +108,6 @@ public class MasterActionListBox extends ActionListBox {
                     Storage.KeyDerivationService keyDerivationService = new Storage.KeyDerivationService(storage, new SecureString(password), true);
                     keyDerivationService.setOnSucceeded(workerStateEvent -> {
                         EventManager.get().post(new StorageEvent(walletId, TimedEvent.Action.END, "Done"));
-                        keyDerivationService.getValue().clear();
                         SparrowTerminal.get().unlockWallet(storage);
                         SparrowTerminal.get().getGuiThread().invokeLater(() -> LoadWallet.getOpeningDialog(storage, wallet).showDialog(SparrowTerminal.get().getGui()));
                     });
diff --git a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/NewWalletDialog.java b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/NewWalletDialog.java
index 945efe1..b02c21c 100644
--- a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/NewWalletDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/NewWalletDialog.java
@@ -172,7 +172,6 @@ public abstract class NewWalletDialog extends DialogWindow {
                         } catch(IOException | StorageException | MnemonicException e) {
                             log.error("Error saving imported wallet", e);
                         } finally {
-                            encryptionFullKey.clear();
                             if(key != null) {
                                 key.clear();
                             }
diff --git a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java
index ca879b9..6bc865f 100644
--- a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/SettingsDialog.java
@@ -148,7 +148,6 @@ public class SettingsDialog extends WalletDialog {
                             copy.decrypt(key);
                             showSuccessDialog("Wallet Seed", copy.getKeystores().get(0).getSeed().getMnemonicString().asString());
                         } finally {
-                            encryptionFullKey.clear();
                             if(key != null) {
                                 key.clear();
                             }
@@ -247,7 +246,6 @@ public class SettingsDialog extends WalletDialog {
                             log.error("Error saving wallet", e);
                             AppServices.showErrorDialog("Error saving wallet", e.getMessage());
                         } finally {
-                            encryptionFullKey.clear();
                             if(key != null) {
                                 key.clear();
                             }
diff --git a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/WalletDialog.java b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/WalletDialog.java
index 517a466..7ef4b7f 100644
--- a/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/WalletDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/terminal/wallet/WalletDialog.java
@@ -83,7 +83,6 @@ public class WalletDialog extends DialogWindow {
                         EventManager.get().post(new StorageEvent(walletId, TimedEvent.Action.END, "Done"));
                         ECKey encryptionFullKey = keyDerivationService.getValue();
                         Key key = new Key(encryptionFullKey.getPrivKeyBytes(), getWalletForm().getStorage().getKeyDeriver().getSalt(), EncryptionType.Deriver.ARGON2);
-                        encryptionFullKey.clear();
                         masterWallet.decrypt(key);
                         addAndEncryptAccount(masterWallet, standardAccount, key);
                         if(postAddition != null) {
diff --git a/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java b/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
index bc95ee9..bbdec7f 100644
--- a/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/wallet/SettingsController.java
@@ -573,7 +573,6 @@ public class SettingsController extends WalletFormController implements Initiali
                     log.error("Error restoring public keys from seed", e);
                 } finally {
                     key.clear();
-                    encryptionFullKey.clear();
                     password.get().clear();
                 }
             });
@@ -680,7 +679,6 @@ public class SettingsController extends WalletFormController implements Initiali
                         EventManager.get().post(new StorageEvent(walletId, TimedEvent.Action.END, "Done"));
                         ECKey encryptionFullKey = keyDerivationService.getValue();
                         Key key = new Key(encryptionFullKey.getPrivKeyBytes(), walletForm.getStorage().getKeyDeriver().getSalt(), EncryptionType.Deriver.ARGON2);
-                        encryptionFullKey.clear();
                         masterWallet.decrypt(key);
 
                         if(masterWallet.getKeystores().stream().anyMatch(ks -> ks.getSource() != KeystoreSource.SW_SEED)) {
@@ -1047,7 +1045,6 @@ public class SettingsController extends WalletFormController implements Initiali
                         revert.setDisable(false);
                         apply.setDisable(false);
                     } finally {
-                        encryptionFullKey.clear();
                         if(key != null) {
                             key.clear();
                         }

Risk score

Why this scored 28/100

Our methodology →

Potential impact 5/30

Exploitability 2/25

Stealth signal 8/15

Affected reach 4/15

Confidence 6/10

Evidence quality 3/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 28/100

Community notes

Related watches

avoid deleting the backups of same-prefixed wallets

verify proof of work on chain tips and warn when a tip goes stale

prevent a password change from re-encrypting wallets whose filenames share the same prefix