← Watch feed
serialise usb device access so the periodic enumeration cannot interrupt an operation in progress
Public commit record
50/100 · Thin
What the developer wrote
Authored by Craig Raw
serialise usb device access so the periodic enumeration cannot interrupt an operation in progress
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit fixes a race condition in how Sparrow Wallet talks to USB hardware wallets. Previously, a background check for connected devices could run at the same time as a user action such as entering a PIN, signing a transaction, or displaying an address. That overlap could interrupt the device mid-operation, clear the PIN screen, or return confusing results. The change wraps all USB device operations with a single lock so they cannot overlap, and the periodic background check now skips its run if any device operation is already in progress.
Recommended action
Treat this as a reliability and likely security-hardening fix. Users relying on hardware wallets with Sparrow should upgrade to a release containing this commit. Developers should verify that the lock is held for the entire duration of each device interaction, including any UI prompts that occur while the lock is held, and consider whether the same issue exists across multiple Sparrow instances or other processes accessing the same USB device.
Security signals we found
01
Race condition between periodic USB enumeration and user-initiated device operations
02
Background enumeration could clear an active PIN prompt on the hardware device
03
Concurrent USB access could corrupt or abort signing, address display, or xpub retrieval
04
Addition of a global ReentrantLock to serialize all USB device operations
05
Periodic enumeration now defers when a device operation is in progress
06
isPromptActive flag management simplified and narrowed
07
Pairing dialog thread now captures DeviceException instead of throwing RuntimeException
Evidence from the diff
The patch serializes access to USB hardware wallets in Hwi.java using a static ReentrantLock (deviceLock). Every public method that performs a device operation—enumerateUsb, promptPin, sendPin, togglePassphrase, getXpubs/getXpub, getSpscan, displayAddress, signMessage, signPSBT—now acquires deviceLock before invoking the underlying Lark library and releases it in a finally block. The periodic ScheduledEnumerateService now uses deviceLock.tryLock() and returns null (skipping the enumeration) if the lock is held, instead of relying solely on isPromptActive. The isPromptActive flag is no longer set inside most methods; it is now only reset in enumerateUsb, sendPin, and the pairing thread, and is removed from pairing-code UI futures. The lark submodule is also bumped. The fix is defensive and partial: it prevents concurrent USB access from the same JVM but does not address other concurrency models or underlying library issues.
Changed components
src/main/java/com/sparrowwallet/sparrow/io/Hwi.javasrc/main/java/com/sparrowwallet/sparrow/AppServices.javalark submoduleInspect captured patch +59 / −32
### lark
@@ -1 +1 @@
-Subproject commit 9ea1b988062612df503ff8da8dffa35eff5b1c63
+Subproject commit 33fbba951e3a881a96a71f4711777dbc69f9c0ab
### src/main/java/com/sparrowwallet/sparrow/AppServices.java
@@ -433,7 +433,7 @@ private Hwi.ScheduledEnumerateService createDeviceEnumerateService() {
enumerateService.setOnSucceeded(workerStateEvent -> {
List<Device> devices = enumerateService.getValue();
- //Null devices are returned if the app is currently prompting for a pin. Otherwise, the enumerate clears the pin screen
+ //Null devices are returned if the app is currently prompting for a pin (the enumerate would clear the pin screen) or another device operation is in progress
if(devices != null) {
//If another instance of HWI is currently accessing the usb interface, HWI returns empty device models. Ignore this run if that happens
List<Device> validDevices = devices.stream().filter(device -> device.getModel() != null).collect(Collectors.toList());
### src/main/java/com/sparrowwallet/sparrow/io/Hwi.java
@@ -41,6 +41,8 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
public class Hwi {
private static final Logger log = LoggerFactory.getLogger(Hwi.class);
@@ -49,6 +51,10 @@ public class Hwi {
private static final String BITBOX_FILENAME = "bitbox02.json";
private static final String TREZOR_FILENAME = "trezor.json";
+ //Serialises all USB device access, including the periodic enumeration which would otherwise interfere with an operation in progress on the same device
+ private static final ReentrantLock deviceLock = new ReentrantLock();
+
+ //Indicates a pin prompt has been shown on a device and not yet answered. Enumerating in this state clears the pin screen
private static volatile boolean isPromptActive = false;
private final Set<byte[]> newDeviceRegistrations = new HashSet<>();
@@ -65,15 +71,16 @@ public List<Device> enumerate(String passphrase) throws ImportException {
}
private List<Device> enumerateUsb(String passphrase) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark(passphrase);
- isPromptActive = true;
return lark.enumerate().stream().map(Device::fromHardwareClient).toList();
} catch(Throwable e) {
log.error("Error enumerating USB devices", e);
throw new ImportException(e.getMessage() == null || e.getMessage().isEmpty() ? "Error scanning, check devices are ready" : e.getMessage(), e);
} finally {
isPromptActive = false;
+ deviceLock.unlock();
}
}
@@ -108,6 +115,7 @@ private List<Device> enumerateCard() {
}
public boolean promptPin(Device device) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark();
boolean result = lark.promptPin(device.getType(), device.getPath());
@@ -118,24 +126,29 @@ public boolean promptPin(Device device) throws ImportException {
} catch(RuntimeException e) {
log.error("Error prompting pin", e);
throw e;
+ } finally {
+ deviceLock.unlock();
}
}
public boolean sendPin(Device device, String pin) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark();
- boolean result = lark.sendPin(device.getType(), device.getPath(), pin);
- isPromptActive = false;
- return result;
+ return lark.sendPin(device.getType(), device.getPath(), pin);
} catch(DeviceException e) {
throw new ImportException(e.getMessage(), e);
} catch(RuntimeException e) {
log.error("Error sending pin", e);
throw e;
+ } finally {
+ isPromptActive = false;
+ deviceLock.unlock();
}
}
public boolean togglePassphrase(Device device) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark();
boolean result = lark.togglePassphrase(device.getType(), device.getPath());
@@ -146,18 +159,26 @@ public boolean togglePassphrase(Device device) throws ImportException {
} catch(RuntimeException e) {
log.error("Error toggling passphrase", e);
throw e;
+ } finally {
+ deviceLock.unlock();
}
}
public Map<WalletType, ExtendedKey> getXpubs(Device device, String passphrase, Map<WalletType, String> accountDerivationPaths, Map<WalletType, ExtendedKey> accountXpubs) throws ImportException {
- for(Map.Entry<WalletType, String> entry : accountDerivationPaths.entrySet()) {
- accountXpubs.put(entry.getKey(), getXpub(device, passphrase, entry.getValue()));
+ deviceLock.lock();
+ try {
+ for(Map.Entry<WalletType, String> entry : accountDerivationPaths.entrySet()) {
+ accountXpubs.put(entry.getKey(), getXpub(device, passphrase, entry.getValue()));
+ }
+ } finally {
+ deviceLock.unlock();
}
return accountXpubs;
}
public ExtendedKey getXpub(Device device, String passphrase, String derivationPath) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark(passphrase);
ExtendedKey xpub = lark.getPubKeyAtPath(device.getType(), device.getPath(), derivationPath);
@@ -168,10 +189,13 @@ public ExtendedKey getXpub(Device device, String passphrase, String derivationPa
} catch(RuntimeException e) {
log.error("Error retrieving xpub", e);
throw e;
+ } finally {
+ deviceLock.unlock();
}
}
public SilentPaymentScanAddress getSpscan(Device device, String passphrase, String derivationPath) throws ImportException {
+ deviceLock.lock();
try {
Lark lark = getLark(passphrase);
SilentPaymentScanAddress spscan = lark.getSpscanAtPath(device.getType(), device.getPath(), derivationPath);
@@ -182,17 +206,19 @@ public SilentPaymentScanAddress getSpscan(Device device, String passphrase, Stri
} catch(RuntimeException e) {
log.error("Error retrieving spscan", e);
throw e;
+ } finally {
+ deviceLock.unlock();
}
}
public String displayAddress(Device device, String passphrase, ScriptType scriptType, OutputDescriptor addressDescriptor,
OutputDescriptor walletDescriptor, String walletName, byte[] walletRegistration) throws DisplayAddressException {
- try {
- if(!Arrays.asList(ScriptType.ADDRESSABLE_TYPES).contains(scriptType)) {
- throw new IllegalArgumentException("Cannot display address for script type " + scriptType + ": Only addressable types supported");
- }
+ if(!Arrays.asList(ScriptType.ADDRESSABLE_TYPES).contains(scriptType)) {
+ throw new IllegalArgumentException("Cannot display address for script type " + scriptType + ": Only addressable types supported");
+ }
- isPromptActive = true;
+ deviceLock.lock();
+ try {
Lark lark = getLark(passphrase, walletDescriptor, walletName, walletRegistration);
String address = lark.displayAddress(device.getType(), device.getPath(), addressDescriptor);
newDeviceRegistrations.addAll(lark.getWalletRegistrations().values());
@@ -204,13 +230,13 @@ public String displayAddress(Device device, String passphrase, ScriptType script
log.error("Error displaying address", e);
throw e;
} finally {
- isPromptActive = false;
+ deviceLock.unlock();
}
}
public String signMessage(Device device, String passphrase, String message, String derivationPath) throws SignMessageException {
+ deviceLock.lock();
try {
- isPromptActive = true;
Lark lark = getLark(passphrase);
return lark.signMessage(device.getType(), device.getPath(), message, derivationPath);
} catch(DeviceException e) {
@@ -219,14 +245,14 @@ public String signMessage(Device device, String passphrase, String message, Stri
log.error("Error signing message", e);
throw e;
} finally {
- isPromptActive = false;
+ deviceLock.unlock();
}
}
public PSBT signPSBT(Device device, String passphrase, PSBT psbt,
OutputDescriptor walletDescriptor, String walletName, byte[] walletRegistration) throws SignTransactionException {
+ deviceLock.lock();
try {
- isPromptActive = true;
Lark lark = getLark(passphrase, walletDescriptor, walletName, walletRegistration);
PSBT signed = lark.signTransaction(device.getType(), device.getPath(), psbt);
newDeviceRegistrations.addAll(lark.getWalletRegistrations().values());
@@ -238,7 +264,7 @@ public PSBT signPSBT(Device device, String passphrase, PSBT psbt,
log.error("Error signing PSBT", e);
throw e;
} finally {
- isPromptActive = false;
+ deviceLock.unlock();
}
}
@@ -311,9 +337,13 @@ public ScheduledEnumerateService(String passphrase) {
protected Task<List<Device>> createTask() {
return new Task<>() {
protected List<Device> call() throws ImportException {
- if(!isPromptActive) {
- Hwi hwi = new Hwi();
- return hwi.enumerate(passphrase);
+ if(!isPromptActive && deviceLock.tryLock()) {
+ try {
+ Hwi hwi = new Hwi();
+ return hwi.enumerate(passphrase);
+ } finally {
+ deviceLock.unlock();
+ }
}
return null;
@@ -569,16 +599,15 @@ public void attestationCheck(boolean result) {
public boolean showPairing(String code, DeviceResponse response) throws DeviceException {
CountDownLatch latch = new CountDownLatch(1);
AtomicBoolean confirmedDevice = new AtomicBoolean(false);
+ AtomicReference<DeviceException> deviceException = new AtomicReference<>();
Thread showPairingDeviceThread = new Thread(() -> {
try {
- isPromptActive = true;
confirmedDevice.set(response.call());
- latch.countDown();
} catch(DeviceException e) {
- throw new RuntimeException(e);
+ deviceException.set(e);
} finally {
- isPromptActive = false;
+ latch.countDown();
}
});
showPairingDeviceThread.start();
@@ -599,11 +628,15 @@ public boolean showPairing(String code, DeviceResponse response) throws DeviceEx
if(pairingDialog != null && pairingDialog.isShowing()) {
pairingDialog.setResult(ButtonType.APPLY);
}
- if(!confirmedDevice.get()) {
+ if(deviceException.get() == null && !confirmedDevice.get()) {
AppServices.showWarningDialog("Pairing Refused", "Pairing was refused on the device.");
}
});
+ if(deviceException.get() != null) {
+ throw deviceException.get();
+ }
+
return confirmedDevice.get();
}
}
@@ -639,13 +672,10 @@ public String promptForPairingCode() {
});
try {
- isPromptActive = true;
return future.get(); // Block until dialog is closed
} catch (InterruptedException | ExecutionException e) {
Thread.currentThread().interrupt();
return null;
- } finally {
- isPromptActive = false;
}
}
@@ -659,13 +689,10 @@ public boolean confirmPairing(String deviceInfo) {
});
try {
- isPromptActive = true;
return future.get() == ButtonType.YES; // Block until dialog is closed
} catch (InterruptedException | ExecutionException e) {
Thread.currentThread().interrupt();
return false;
- } finally {
- isPromptActive = false;
}
}
Risk score
Our methodology →Why this scored 42/100
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.