Skip to content

Commit 5877823

Browse files
fix: zero password buffers and rename misleading APDU constant (#932)
* fix: ledger app-not-open detection and quiet-flag leak - Split LedgerAddressUtil.getTronAddress into getRawAddressResponse + parseTronAddress so callers can inspect APDU status words before parsing - Add LedgerPorts.AppNotOpenException for APDU 0x6511 (Tron app not open), distinct from a null return (device not found / address mismatch) - NonInteractiveLedgerSigner catches AppNotOpenException and returns APP_NOT_OPEN outcome instead of NOT_CONNECTED - ProductionLedgerPorts uses try-finally to unconditionally reset standardCliQuiet after executeSignListen returns, fixing permanent stdout suppression when device times out without a HID callback - AliasResolutionException overrides fillInStackTrace as a no-op to avoid unnecessary stack capture on control-flow exceptions Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: correctly detect app-not-open when Ledger HID open() fails On some OS/firmware combinations, HidDevice.open() returns false when the Tron app is not running, causing getLedgerHidDevice() to return null and the signer to report NOT_CONNECTED instead of APP_NOT_OPEN. - Add HidServicesWrapper.hasAnyLedgerAttached() to distinguish "no device" from "device present but not openable" - Throw AppNotOpenException in ProductionLedgerPorts when getHidDevice() returns null but a Ledger is physically attached - Also check the return value of the second device.open() call (B2 path) which was previously ignored, causing the same misclassification - Add test: returnsAppNotOpenWhenFinderThrowsAppNotOpenException Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: zero password buffers in StdinPasswordReader and rename misleading APDU constant - StdinPasswordReader.readAll wraps the read/parse in try-finally and Arrays.fill the chunk and bytes buffers before returning, matching the defensive pattern already used in StandardCliRunner.authenticate. The returned String still holds the password in its own char[], but the intermediate byte arrays no longer linger on the heap until GC. - Rename APDU_APP_IS_OPEN to APDU_APP_NOT_OPEN. The Javadoc and the error message ("Open the Tron app on your Ledger device") already treat 0x6511 as the "not open" signal; the identifier now matches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Will <> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 4561b70 commit 5877823

8 files changed

Lines changed: 115 additions & 31 deletions

File tree

src/main/java/org/tron/ledger/LedgerAddressUtil.java

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ public static Map<String, String> getMultiImportAddress(List<String> paths, HidD
7373
return addressMap;
7474
}
7575

76-
public static String getTronAddress(String path, HidDevice hidDevice) {
76+
/**
77+
* Sends the "get address" APDU and returns the raw response bytes without parsing.
78+
* Returns {@code null} on transport failure. Callers can inspect error status words
79+
* (e.g. {@code 0x6511} = Tron app not open) before falling through to address parsing.
80+
*/
81+
public static byte[] getRawAddressResponse(String path, HidDevice hidDevice) {
7782
try {
7883
byte[] apdu = ApduMessageBuilder.buildTronAddressApduMessage(path);
7984
if (DebugConfig.isDebugEnabled()) {
@@ -83,11 +88,25 @@ public static String getTronAddress(String path, HidDevice hidDevice) {
8388
if (DebugConfig.isDebugEnabled()) {
8489
System.out.println("Get Address Response: " + CommonUtil.bytesToHex(result));
8590
}
86-
if (LedgerConstant.LEDGER_LOCK.equalsIgnoreCase(CommonUtil.bytesToHex(result))) {
87-
System.out.println(ANSI_RED + "Ledger is locked, please unlock it first"+ ANSI_RESET);
88-
return EMPTY;
91+
return result;
92+
} catch (Exception e) {
93+
if (DebugConfig.isDebugEnabled()) {
94+
e.printStackTrace();
8995
}
96+
return null;
97+
}
98+
}
9099

100+
/** Parses a Tron Base58 address from a raw "get address" APDU response. Returns {@code ""} on any parse failure. */
101+
public static String parseTronAddress(byte[] result) {
102+
if (result == null || result.length < 2) {
103+
return EMPTY;
104+
}
105+
if (LedgerConstant.LEDGER_LOCK.equalsIgnoreCase(CommonUtil.bytesToHex(result))) {
106+
System.out.println(ANSI_RED + "Ledger is locked, please unlock it first" + ANSI_RESET);
107+
return EMPTY;
108+
}
109+
try {
91110
int offset = 0;
92111
int publicKeyLength = result[offset++] & 0xFF;
93112
byte[] publicKey = new byte[publicKeyLength];
@@ -98,6 +117,18 @@ public static String getTronAddress(String path, HidDevice hidDevice) {
98117
byte[] addressBytes = new byte[addressLength];
99118
System.arraycopy(result, offset, addressBytes, 0, addressLength);
100119
return new String(addressBytes);
120+
} catch (Exception e) {
121+
if (DebugConfig.isDebugEnabled()) {
122+
e.printStackTrace();
123+
}
124+
return EMPTY;
125+
}
126+
}
127+
128+
public static String getTronAddress(String path, HidDevice hidDevice) {
129+
try {
130+
byte[] result = getRawAddressResponse(path, hidDevice);
131+
return parseTronAddress(result);
101132
} catch (Exception e) {
102133
System.err.println("Error: " + e.getMessage());
103134
if (DebugConfig.isDebugEnabled()) {

src/main/java/org/tron/ledger/wrapper/HidServicesWrapper.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ public HidServices initHidServices() {
6262
return hs;
6363
}
6464

65+
public boolean hasAnyLedgerAttached() {
66+
return getHidServices().getAttachedHidDevices().stream()
67+
.anyMatch(d -> d.getVendorId() == LEDGER_VENDOR_ID);
68+
}
69+
6570
public static HidDevice getLedgerHidDevice(HidServices hidServices, String address, String path) {
6671
List<HidDevice> hidDeviceList = new ArrayList<>();
6772
HidDevice fidoDevice = null;

src/main/java/org/tron/walletcli/cli/StdinPasswordReader.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.io.IOException;
55
import java.io.InputStream;
66
import java.nio.charset.StandardCharsets;
7+
import java.util.Arrays;
78

89
/**
910
* Reads MASTER_PASSWORD from an {@link InputStream} (typically {@code System.in}) once and caches
@@ -37,28 +38,34 @@ public synchronized String get() {
3738
private String readAll() {
3839
ByteArrayOutputStream buf = new ByteArrayOutputStream();
3940
byte[] chunk = new byte[256];
41+
byte[] bytes = null;
4042
try {
4143
int n;
4244
while ((n = in.read(chunk)) != -1) {
4345
buf.write(chunk, 0, n);
4446
}
47+
if (buf.size() == 0) {
48+
return null;
49+
}
50+
bytes = buf.toByteArray();
51+
int len = bytes.length;
52+
if (len > 0 && bytes[len - 1] == '\n') {
53+
len--;
54+
if (len > 0 && bytes[len - 1] == '\r') {
55+
len--;
56+
}
57+
}
58+
if (len == 0) {
59+
return null;
60+
}
61+
return new String(bytes, 0, len, StandardCharsets.UTF_8);
4562
} catch (IOException e) {
4663
throw new IllegalStateException("Failed to read password from stdin: " + e.getMessage(), e);
47-
}
48-
if (buf.size() == 0) {
49-
return null;
50-
}
51-
byte[] bytes = buf.toByteArray();
52-
int len = bytes.length;
53-
if (len > 0 && bytes[len - 1] == '\n') {
54-
len--;
55-
if (len > 0 && bytes[len - 1] == '\r') {
56-
len--;
64+
} finally {
65+
Arrays.fill(chunk, (byte) 0);
66+
if (bytes != null) {
67+
Arrays.fill(bytes, (byte) 0);
5768
}
5869
}
59-
if (len == 0) {
60-
return null;
61-
}
62-
return new String(bytes, 0, len, StandardCharsets.UTF_8);
6370
}
6471
}

src/main/java/org/tron/walletcli/cli/aliases/AliasResolutionException.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,9 @@ public class AliasResolutionException extends IllegalArgumentException {
44
public AliasResolutionException(String message) {
55
super(message);
66
}
7+
8+
@Override
9+
public Throwable fillInStackTrace() {
10+
return this;
11+
}
712
}

src/main/java/org/tron/walletcli/cli/ledger/LedgerPorts.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,15 @@ public interface SignResultReader {
9393
public interface ContractSupport {
9494
boolean canSign(Chain.Transaction transaction);
9595
}
96+
97+
/**
98+
* Thrown by {@link HidDeviceFinder#find} when the Ledger device is physically connected
99+
* but the Tron app is not open (APDU status word {@code 0x6511}).
100+
* Distinct from a plain {@code null} return (device not found / address mismatch).
101+
*/
102+
public static final class AppNotOpenException extends RuntimeException {
103+
public AppNotOpenException(String message) {
104+
super(message);
105+
}
106+
}
96107
}

src/main/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSigner.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public final class NonInteractiveLedgerSigner implements LedgerSigner {
2929
static final String STATE_TIMEOUT = "timeout"; // SIGN_RESULT_TIMEOUT — timed out
3030

3131
/** APDU status word: Tron app is not open on the device. */
32-
private static final byte[] APDU_APP_IS_OPEN = new byte[] { 0x65, 0x11 };
32+
private static final byte[] APDU_APP_NOT_OPEN = new byte[] { 0x65, 0x11 };
3333
/** APDU status word: "Sign By Hash" setting is not enabled. */
3434
private static final byte[] APDU_SIGN_BY_HASH = new byte[] { 0x6a, (byte) 0x8c };
3535

@@ -68,6 +68,9 @@ public LedgerSignOutcome sign(Chain.Transaction transaction,
6868
try (SystemOutSuppressor ignored = SystemOutSuppressor.capture()) {
6969
try {
7070
device = finder.find(address, bip44Path);
71+
} catch (LedgerPorts.AppNotOpenException e) {
72+
return LedgerSignOutcome.failure(LedgerSignOutcome.Status.APP_NOT_OPEN,
73+
e.getMessage());
7174
} catch (RuntimeException e) {
7275
return LedgerSignOutcome.failure(LedgerSignOutcome.Status.NOT_CONNECTED,
7376
"HID transport failure: " + e.getMessage());
@@ -102,7 +105,7 @@ public LedgerSignOutcome sign(Chain.Transaction transaction,
102105

103106
byte[] apdu = executor.lastSendResultBytes();
104107
if (apdu != null && apdu.length > 0) {
105-
if (matches(apdu, APDU_APP_IS_OPEN)) {
108+
if (matches(apdu, APDU_APP_NOT_OPEN)) {
106109
stateReader.markCanceled(device.path(), txid);
107110
return LedgerSignOutcome.failure(LedgerSignOutcome.Status.APP_NOT_OPEN,
108111
"Open the Tron app on your Ledger device and try again");

src/main/java/org/tron/walletcli/cli/ledger/ProductionLedgerPorts.java

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,28 @@ public static NonInteractiveLedgerSigner buildSigner(OutputFormatter formatter)
2424
LedgerPorts.HidDeviceFinder finder = (address, path) -> {
2525
HidDevice device = HidServicesWrapper.getInstance().getHidDevice(address, path);
2626
if (device == null) {
27+
if (HidServicesWrapper.getInstance().hasAnyLedgerAttached()) {
28+
throw new LedgerPorts.AppNotOpenException(
29+
"Open the Tron app on your Ledger device and try again");
30+
}
2731
return null;
2832
}
2933
boolean matched = false;
3034
try {
31-
if (device.isClosed()) {
32-
device.open();
35+
if (device.isClosed() && !device.open()) {
36+
throw new LedgerPorts.AppNotOpenException(
37+
"Open the Tron app on your Ledger device and try again");
38+
}
39+
byte[] rawResponse = LedgerAddressUtil.getRawAddressResponse(path, device);
40+
// 0x6511: Tron app is not open on the device (ISO 7816-4 "conditions not satisfied").
41+
// Distinguish from a genuine address mismatch so the caller can surface the right error.
42+
if (rawResponse != null && rawResponse.length == 2
43+
&& (rawResponse[0] & 0xFF) == 0x65 && (rawResponse[1] & 0xFF) == 0x11) {
44+
throw new LedgerPorts.AppNotOpenException(
45+
"Open the Tron app on your Ledger device and try again");
3346
}
34-
String deviceAddress = LedgerAddressUtil.getTronAddress(path, device);
35-
matched = address.equals(deviceAddress);
47+
String deviceAddress = LedgerAddressUtil.parseTronAddress(rawResponse);
48+
matched = address.equals(deviceAddress) && !deviceAddress.isEmpty();
3649
if (!matched) {
3750
return null;
3851
}
@@ -87,14 +100,12 @@ public boolean executeSignListen(LedgerPorts.DeviceHandle device, Chain.Transact
87100
if (raw.isClosed()) {
88101
raw.open();
89102
}
90-
boolean accepted = listener.executeSignListen(raw, tx, path, gasfree);
91-
if (listener.getLastSendResultBytes() != null || !accepted) {
92-
listener.setStandardCliQuiet(false);
93-
}
94-
return accepted;
95-
} catch (RuntimeException e) {
103+
return listener.executeSignListen(raw, tx, path, gasfree);
104+
} finally {
105+
// Always reset: executeSignListen blocks until the 60-second wait completes,
106+
// so by the time we return, the HID callback has either already reset this flag
107+
// or it never will (silent timeout / device disconnect).
96108
listener.setStandardCliQuiet(false);
97-
throw e;
98109
}
99110
}
100111

src/test/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSignerTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,17 @@ public void returnsNotConnectedWhenFinderThrows() {
103103
Assert.assertTrue(r.getMessage().contains("transport boom"));
104104
}
105105

106+
@Test
107+
public void returnsAppNotOpenWhenFinderThrowsAppNotOpenException() {
108+
// Simulates ProductionLedgerPorts detecting that a Ledger is physically attached
109+
// (hasAnyLedgerAttached = true) but HID open() failed because the Tron app is not running.
110+
finder.toThrow = new LedgerPorts.AppNotOpenException(
111+
"Open the Tron app on your Ledger device and try again");
112+
LedgerSignOutcome r = signNonGasfree();
113+
Assert.assertEquals(LedgerSignOutcome.Status.APP_NOT_OPEN, r.getStatus());
114+
Assert.assertTrue(r.getMessage().contains("Tron app"));
115+
}
116+
106117
@Test
107118
public void returnsUnsupportedContractBeforeDeviceLookup() {
108119
contractSupport.canSign = false;

0 commit comments

Comments
 (0)