fail rather than loop indefinitely on an unsuccessful card wait or repeated secure channel resets
What the developer wrote
Authored by Craig Raw
fail rather than loop indefinitely on an unsuccessful card wait or repeated secure channel resets
What changed, and why it matters
This commit fixes two situations where Sparrow Wallet could get stuck in an endless loop while talking to a hardware card (Coldcard/Satochip). Previously, if the card refused a 'wait' request or repeatedly rejected the secure channel setup, the software would retry forever. Now it throws an error or stops after three retries. This is a reliability/DoS fix rather than a direct theft-of-funds bug, but an attacker who can make the card misbehave could freeze the wallet interface.
Treat as a low-to-moderate reliability/security fix. Users relying on Coldcard or Satochip devices should upgrade. Review the lark submodule diff for related changes, and verify that returning an empty APDUResponse after max secure-channel resets is handled safely by callers (e.g., does not cause silent failures or incorrect state).
Security signals we found
Infinite loop / denial-of-service condition in card communication
Missing failure handling for unsuccessful card wait response
Missing retry limit on secure channel reset
Hardware wallet integration code modified
Subproject dependency updated (lark)
Evidence from the diff
The patch adds termination conditions to two hardware-card communication paths. In CkCardApi.checkWait(), if cardProtocol.authWait() returns success=false, the code now throws a CardException instead of leaving the previous delay value and looping. In SatochipCommandSet.cardTransmit(), a counter limits secure-channel resets to MAX_SECURE_CHANNEL_RESETS (3); after that, the method returns an empty APDUResponse rather than looping on 0x9C21. The lark submodule bump likely contains related fixes. These changes prevent infinite retry loops that could hang the application or keep the secure channel in a broken state.
Changed components
src/main/java/com/sparrowwallet/sparrow/io/ckcard/CkCardApi.javasrc/main/java/com/sparrowwallet/sparrow/io/satochip/SatochipCommandSet.javalark (submodule)Inspect captured patch +13 / −3
### lark
@@ -1 +1 @@
-Subproject commit f2b6b6624752bf195d9d506f7ee07d2e65a90a03
+Subproject commit 450758a05e80fcc5c3a75131840f1884a06a4332
### src/main/java/com/sparrowwallet/sparrow/io/ckcard/CkCardApi.java
@@ -83,9 +83,11 @@ void checkWait(CardStatus cardStatus, IntegerProperty delayProperty, StringPrope
delayProperty.set(delay);
messageProperty.set("Auth delay, waiting " + delay + "s...");
CardWait cardWait = cardProtocol.authWait();
- if(cardWait.success) {
- delay = cardWait.auth_delay == null ? 0 : cardWait.auth_delay.intValue();
+ if(!cardWait.success) {
+ throw new CardException("Card did not accept the request to wait out the authentication delay.");
}
+
+ delay = cardWait.auth_delay == null ? 0 : cardWait.auth_delay.intValue();
}
}
}
### src/main/java/com/sparrowwallet/sparrow/io/satochip/SatochipCommandSet.java
@@ -27,6 +27,8 @@ public class SatochipCommandSet {
private static final Logger log = LoggerFactory.getLogger(SatochipCommandSet.class);
+ private static final int MAX_SECURE_CHANNEL_RESETS = 3;
+
private final SatoCardTransport cardTransport;
private final SecureChannelSession secureChannel;
private SatoCardStatus status;
@@ -66,6 +68,7 @@ public SatoCardStatus getApplicationStatus() {
public APDUResponse cardTransmit(APDUCommand plainApdu) {
// we try to transmit the APDU until we receive the answer or we receive an unrecoverable error
boolean isApduTransmitted = false;
+ int secureChannelResets = 0;
do {
try {
byte[] apduBytes = plainApdu.serialize();
@@ -114,6 +117,11 @@ else if(sw12 == 0x9C06) {
// SecureChannel is not initialized
else if(sw12 == 0x9C21) {
log.error("Error, Satochip secure channel required");
+ if(++secureChannelResets > MAX_SECURE_CHANNEL_RESETS) {
+ // the card keeps asking for a secure channel it will not accept, so stop rather than retry indefinitely
+ log.error("Error, Satochip secure channel could not be established");
+ return new APDUResponse(new byte[0], (byte)0x00, (byte)0x00);
+ }
secureChannel.resetSecureChannel();
} else {
// cannot resolve issue at this pointWhy this scored 48/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.