pass default derivation to usb device and card import panes

Sparrow/Sparrow Wallet

8efee4ea View source ↗

Public commit record

What the developer wrote

Authored by Craig Raw

50/100 · Thin

pass default derivation to usb device and card import panes

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

The short version

What changed, and why it matters

This commit changes how Sparrow Wallet picks the default Bitcoin key derivation path when importing from a hardware wallet or card. Previously, the wallet always used the default path for the wallet's script type. Now it can accept and use a 'default derivation' suggested by the caller, falling back to the script type default only if none is provided. The change also fixes a potential null-pointer issue when no wallet or script type is available. There is no direct evidence in the commit of a security vulnerability being fixed; it reads more like a correctness or feature improvement for derivation-path handling.

Recommended action

Treat as a routine functional/correctness update. Review that the new defaultDerivation parameter is validated and cannot be manipulated by untrusted input to cause derivation-path confusion. If this commit is part of a larger release, check the project changelog or release notes for any security-related context not present in the commit itself.

Security signals we found

Change in key derivation path selection logic for hardware wallet and card imports

Addition of null-safe fallback to P2WPKH default derivation when wallet or script type is missing

Separation of 'default derivation' from 'required derivation' in UI controller constructors

No mention of vulnerability, CVE, security researcher, or exploit in commit message or diff

Technical analysis

Evidence from the diff

The patch refactors CardImportPane and DevicePane constructors to accept an additional KeyDerivation parameter (defaultDerivation) separate from requiredDerivation. It introduces helper methods getDefaultDerivation() that prefer the supplied defaultDerivation if non-null and non-empty, otherwise fall back to wallet.getScriptType().getDefaultDerivation(), and finally to ScriptType.P2WPKH.getDefaultDerivation() when wallet or scriptType is null. Call sites in HwAirgappedController, HwUsbDevicesController, WalletImportDialog, and KeystoreImportDialog are updated to pass the new argument. The DevicePane field keyDerivation is split into defaultDerivation and requiredDerivation, and all references are updated accordingly. No explicit security bug, CVE, or advisory is mentioned.

Changed components

com.sparrowwallet.sparrow.control.CardImportPanecom.sparrowwallet.sparrow.control.DevicePanecom.sparrowwallet.sparrow.control.WalletImportDialogcom.sparrowwallet.sparrow.keystoreimport.HwAirgappedControllercom.sparrowwallet.sparrow.keystoreimport.HwUsbDevicesControllercom.sparrowwallet.sparrow.keystoreimport.KeystoreImportDialog

Inspect captured patch +45 / −31

diff --git a/src/main/java/com/sparrowwallet/sparrow/control/CardImportPane.java b/src/main/java/com/sparrowwallet/sparrow/control/CardImportPane.java
index 03ea399..92d7f0d 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/CardImportPane.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/CardImportPane.java
@@ -3,6 +3,7 @@ package com.sparrowwallet.sparrow.control;
 import com.google.common.base.Throwables;
 import com.sparrowwallet.drongo.KeyDerivation;
 import com.sparrowwallet.drongo.crypto.ChildNumber;
+import com.sparrowwallet.drongo.protocol.ScriptType;
 import com.sparrowwallet.drongo.protocol.Sha256Hash;
 import com.sparrowwallet.drongo.wallet.*;
 import com.sparrowwallet.sparrow.AppServices;
@@ -47,10 +48,18 @@ public class CardImportPane extends TitledDescriptionPane {
     protected Button importButton;
     private final SimpleStringProperty pin = new SimpleStringProperty("");
 
-    public CardImportPane(Wallet wallet, KeystoreCardImport importer, KeyDerivation requiredDerivation) {
+    public CardImportPane(Wallet wallet, KeystoreCardImport importer, KeyDerivation defaultDerivation, KeyDerivation requiredDerivation) {
         super(importer.getName(), "Place card on reader", importer.getKeystoreImportDescription(getAccount(wallet, requiredDerivation)), importer.getWalletModel());
         this.importer = importer;
-        this.derivation = requiredDerivation == null ? wallet.getScriptType().getDefaultDerivation() : requiredDerivation.getDerivation();
+        this.derivation = requiredDerivation == null ? getDefaultDerivation(wallet, defaultDerivation) : requiredDerivation.getDerivation();
+    }
+
+    private static List<ChildNumber> getDefaultDerivation(Wallet wallet, KeyDerivation defaultDerivation) {
+        if(defaultDerivation != null && !defaultDerivation.getDerivation().isEmpty()) {
+            return defaultDerivation.getDerivation();
+        }
+
+        return wallet == null || wallet.getScriptType() == null ? ScriptType.P2WPKH.getDefaultDerivation() : wallet.getScriptType().getDefaultDerivation();
     }
 
     @Override
diff --git a/src/main/java/com/sparrowwallet/sparrow/control/DevicePane.java b/src/main/java/com/sparrowwallet/sparrow/control/DevicePane.java
index a1ee742..cd82c54 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/DevicePane.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/DevicePane.java
@@ -51,7 +51,8 @@ public class DevicePane extends TitledDescriptionPane {
     private final Wallet wallet;
     private final PSBT psbt;
     private final OutputDescriptor outputDescriptor;
-    private final KeyDerivation keyDerivation;
+    private final KeyDerivation defaultDerivation;
+    private final KeyDerivation requiredDerivation;
     private final String message;
     private final List<StandardAccount> availableAccounts;
     private final Device device;
@@ -74,13 +75,14 @@ public class DevicePane extends TitledDescriptionPane {
 
     private boolean defaultDevice;
 
-    public DevicePane(Wallet wallet, Device device, boolean defaultDevice, KeyDerivation requiredDerivation) {
+    public DevicePane(Wallet wallet, Device device, boolean defaultDevice, KeyDerivation defaultDerivation, KeyDerivation requiredDerivation) {
         super(device.getModel().toDisplayString(), "", "", device.getModel());
         this.deviceOperation = DeviceOperation.IMPORT;
         this.wallet = wallet;
         this.psbt = null;
         this.outputDescriptor = null;
-        this.keyDerivation = requiredDerivation;
+        this.defaultDerivation = defaultDerivation;
+        this.requiredDerivation = requiredDerivation;
         this.message = null;
         this.availableAccounts = null;
         this.device = device;
@@ -107,7 +109,8 @@ public class DevicePane extends TitledDescriptionPane {
         this.wallet = wallet;
         this.psbt = psbt;
         this.outputDescriptor = null;
-        this.keyDerivation = null;
+        this.defaultDerivation = null;
+        this.requiredDerivation = null;
         this.message = null;
         this.availableAccounts = null;
         this.device = device;
@@ -134,7 +137,8 @@ public class DevicePane extends TitledDescriptionPane {
         this.wallet = wallet;
         this.psbt = null;
         this.outputDescriptor = outputDescriptor;
-        this.keyDerivation = null;
+        this.defaultDerivation = null;
+        this.requiredDerivation = null;
         this.message = null;
         this.availableAccounts = null;
         this.device = device;
@@ -151,13 +155,14 @@ public class DevicePane extends TitledDescriptionPane {
         buttonBox.getChildren().addAll(setPassphraseButton, displayAddressButton);
     }
 
-    public DevicePane(Wallet wallet, String message, KeyDerivation keyDerivation, Device device, boolean defaultDevice) {
+    public DevicePane(Wallet wallet, String message, KeyDerivation requiredDerivation, Device device, boolean defaultDevice) {
         super(device.getModel().toDisplayString(), "", "", device.getModel());
         this.deviceOperation = DeviceOperation.SIGN_MESSAGE;
         this.wallet = wallet;
         this.psbt = null;
         this.outputDescriptor = null;
-        this.keyDerivation = keyDerivation;
+        this.defaultDerivation = requiredDerivation;
+        this.requiredDerivation = requiredDerivation;
         this.message = message;
         this.availableAccounts = null;
         this.device = device;
@@ -184,7 +189,8 @@ public class DevicePane extends TitledDescriptionPane {
         this.wallet = wallet;
         this.psbt = null;
         this.outputDescriptor = null;
-        this.keyDerivation = null;
+        this.defaultDerivation = null;
+        this.requiredDerivation = null;
         this.message = null;
         this.device = device;
         this.defaultDevice = defaultDevice;
@@ -207,7 +213,8 @@ public class DevicePane extends TitledDescriptionPane {
         this.wallet = null;
         this.psbt = null;
         this.outputDescriptor = null;
-        this.keyDerivation = null;
+        this.defaultDerivation = null;
+        this.requiredDerivation = null;
         this.message = null;
         this.device = device;
         this.defaultDevice = defaultDevice;
@@ -286,13 +293,12 @@ public class DevicePane extends TitledDescriptionPane {
     }
 
     private void createImportButton() {
-        importButton = keyDerivation == null ? new SplitMenuButton() : new Button();
+        importButton = requiredDerivation == null ? new SplitMenuButton() : new Button();
         importButton.setAlignment(Pos.CENTER_RIGHT);
         importButton.setText("Import Keystore");
         importButton.setOnAction(event -> {
             importButton.setDisable(true);
-            List<ChildNumber> defaultDerivation = wallet.getScriptType() == null ? ScriptType.P2WPKH.getDefaultDerivation() : wallet.getScriptType().getDefaultDerivation();
-            importKeystore(keyDerivation == null ? defaultDerivation : keyDerivation.getDerivation());
+            importKeystore(requiredDerivation == null ? getDefaultDerivation() : requiredDerivation.getDerivation());
         });
 
         if(importButton instanceof SplitMenuButton importMenuButton) {
@@ -363,7 +369,7 @@ public class DevicePane extends TitledDescriptionPane {
         signMessageButton.managedProperty().bind(signMessageButton.visibleProperty());
         signMessageButton.setVisible(false);
 
-        if(device.getFingerprint() != null && !device.getFingerprint().equals(keyDerivation.getMasterFingerprint())) {
+        if(device.getFingerprint() != null && !device.getFingerprint().equals(requiredDerivation.getMasterFingerprint())) {
             signMessageButton.setDisable(true);
         }
     }
@@ -433,6 +439,14 @@ public class DevicePane extends TitledDescriptionPane {
         getAddressButton.setVisible(false);
     }
 
+    private List<ChildNumber> getDefaultDerivation() {
+        if(defaultDerivation != null && !defaultDerivation.getDerivation().isEmpty()) {
+            return defaultDerivation.getDerivation();
+        }
+
+        return wallet == null || wallet.getScriptType() == null ? ScriptType.P2WPKH.getDefaultDerivation() : wallet.getScriptType().getDefaultDerivation();
+    }
+
     private void unlock(Device device) {
         if(device.getModel().requiresPinPrompt()) {
             promptPin();
@@ -864,7 +878,7 @@ public class DevicePane extends TitledDescriptionPane {
         if(device.isCard()) {
             try {
                 CardApi cardApi = CardApi.getCardApi(device.getModel(), pin.get());
-                Service<String> signMessageService = cardApi.getSignMessageService(message, wallet.getScriptType(), keyDerivation.getDerivation(), messageProperty);
+                Service<String> signMessageService = cardApi.getSignMessageService(message, wallet.getScriptType(), requiredDerivation.getDerivation(), messageProperty);
                 handleCardOperation(signMessageService, signMessageButton, "Signing", true, event -> {
                     String signature = signMessageService.getValue();
                     EventManager.get().post(new MessageSignedEvent(wallet, signature));
@@ -875,7 +889,7 @@ public class DevicePane extends TitledDescriptionPane {
                 signButton.setDisable(false);
             }
         } else {
-            Hwi.SignMessageService signMessageService = new Hwi.SignMessageService(device, passphrase.get(), message, keyDerivation.getDerivationPath());
+            Hwi.SignMessageService signMessageService = new Hwi.SignMessageService(device, passphrase.get(), message, requiredDerivation.getDerivationPath());
             signMessageService.setOnSucceeded(successEvent -> {
                 String signature = signMessageService.getValue();
                 EventManager.get().post(new MessageSignedEvent(wallet, signature));
@@ -1003,8 +1017,7 @@ public class DevicePane extends TitledDescriptionPane {
             importButton.setVisible(true);
             showHideLink.setText("Show derivation...");
             showHideLink.setVisible(!device.isCard());
-            List<ChildNumber> defaultDerivation = wallet.getScriptType() == null ? ScriptType.P2WPKH.getDefaultDerivation() : wallet.getScriptType().getDefaultDerivation();
-            setContent(getDerivationEntry(keyDerivation == null ? defaultDerivation : keyDerivation.getDerivation()));
+            setContent(getDerivationEntry(requiredDerivation == null ? getDefaultDerivation() : requiredDerivation.getDerivation()));
         } else if(deviceOperation.equals(DeviceOperation.SIGN)) {
             signButton.setDefaultButton(defaultDevice);
             signButton.setVisible(true);
@@ -1038,7 +1051,7 @@ public class DevicePane extends TitledDescriptionPane {
         TextField derivationField = new TextField();
         derivationField.setPromptText("Derivation path");
         derivationField.setText(KeyDerivation.writePath(derivation));
-        derivationField.setDisable(device.isCard() || keyDerivation != null);
+        derivationField.setDisable(device.isCard() || requiredDerivation != null);
         HBox.setHgrow(derivationField, Priority.ALWAYS);
 
         ValidationSupport validationSupport = new ValidationSupport();
diff --git a/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java b/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java
index 207418f..dcdef39 100644
--- a/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/control/WalletImportDialog.java
@@ -112,7 +112,7 @@ public class WalletImportDialog extends Dialog<Wallet> {
             List<Device> devices = enumerateService.getValue();
             importAccordion.getPanes().removeIf(titledPane -> titledPane instanceof DevicePane);
             for(Device device : devices) {
-                DevicePane devicePane = new DevicePane(new Wallet(), device, devices.size() == 1, null);
+                DevicePane devicePane = new DevicePane(new Wallet(), device, devices.size() == 1, null, null);
                 importAccordion.getPanes().add(0, devicePane);
             }
             Platform.runLater(() -> EventManager.get().post(new UsbDeviceEvent(devices)));
diff --git a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
index f18588d..063e028 100644
--- a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwAirgappedController.java
@@ -43,7 +43,7 @@ public class HwAirgappedController extends KeystoreImportDetailController {
         List<KeystoreCardImport> cardImporters = List.of(new Tapsigner(), new Satochip(), new Satschip());
         for(KeystoreCardImport importer : cardImporters) {
             if(!importer.isDeprecated() || Config.get().isShowDeprecatedImportExport()) {
-                CardImportPane importPane = new CardImportPane(getMasterController().getWallet(), importer, getMasterController().getRequiredDerivation());
+                CardImportPane importPane = new CardImportPane(getMasterController().getWallet(), importer, getMasterController().getDefaultDerivation(), getMasterController().getRequiredDerivation());
                 if(getMasterController().getRequiredModel() == null || getMasterController().getRequiredModel() == importer.getWalletModel()) {
                     importAccordion.getPanes().add(importPane);
                 }
diff --git a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwUsbDevicesController.java b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwUsbDevicesController.java
index c99f9a9..b1743c0 100644
--- a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwUsbDevicesController.java
+++ b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/HwUsbDevicesController.java
@@ -13,7 +13,7 @@ public class HwUsbDevicesController extends KeystoreImportDetailController {
 
     public void initializeView(List<Device> devices) {
         for(Device device : devices) {
-            DevicePane devicePane = new DevicePane(getMasterController().getWallet(), device, devices.size() == 1, getMasterController().getRequiredDerivation());
+            DevicePane devicePane = new DevicePane(getMasterController().getWallet(), device, devices.size() == 1, getMasterController().getDefaultDerivation(), getMasterController().getRequiredDerivation());
             if(getMasterController().getRequiredModel() == null || getMasterController().getRequiredModel() == device.getModel()) {
                 deviceAccordion.getPanes().add(devicePane);
             }
diff --git a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java
index 14c322f..cacbb82 100644
--- a/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java
+++ b/src/main/java/com/sparrowwallet/sparrow/keystoreimport/KeystoreImportDialog.java
@@ -25,14 +25,6 @@ public class KeystoreImportDialog extends Dialog<Keystore> {
     private final ScriptType scriptType;
     private final String existingLabel;
 
-    public KeystoreImportDialog(Wallet wallet) {
-        this(wallet, KeystoreSource.HW_USB);
-    }
-
-    public KeystoreImportDialog(Wallet wallet, KeystoreSource initialSource) {
-        this(wallet, initialSource, null, null, Keystore.DEFAULT_LABEL, false);
-    }
-
     public KeystoreImportDialog(Wallet wallet, KeystoreSource initialSource, KeyDerivation currentDerivation, WalletModel currentModel, String currentLabel, boolean restrictImport) {
         EventManager.get().register(this);
         setOnCloseRequest(event -> {

Risk score

Why this scored 27/100

Our methodology →

Potential impact 5/30

Exploitability 3/25

Stealth signal 4/15

Affected reach 6/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 27/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