fix: harden standard cli ledger and aliases

gummy789j · gummy789j · commit a4befc59a430 · 2026-05-14T00:49:43.000+08:00
diff --git a/src/main/java/org/tron/ledger/listener/LedgerEventListener.java b/src/main/java/org/tron/ledger/listener/LedgerEventListener.java
@@ -220,7 +220,7 @@ public void hidDataReceived(HidServicesEvent event) {
           println("This transaction has expired, please resign and submit again.");
           LedgerSignResult.updateState(
               TransactionSignManager.getInstance().getHidDevice().getPath()
-              , transactionId, LedgerSignResult.SIGN_RESULT_CANCEL
+              , transactionId, LedgerSignResult.SIGN_RESULT_TIMEOUT
           );
         }
         doLedgerSignEnd();
diff --git a/src/main/java/org/tron/ledger/wrapper/LedgerSignResult.java b/src/main/java/org/tron/ledger/wrapper/LedgerSignResult.java
@@ -17,6 +17,7 @@ public class LedgerSignResult {
   public static final String SIGN_RESULT_SIGNING = "signing";
   public static final String SIGN_RESULT_SUCCESS = "confirmed";
   public static final String SIGN_RESULT_CANCEL = "cancel";
+  public static final String SIGN_RESULT_TIMEOUT = "timeout";
 
   private static final ReadWriteLock lock = new ReentrantReadWriteLock();
   private static final String DIRECTORY = "Ledger";
diff --git a/src/main/java/org/tron/walletcli/cli/aliases/AliasEntry.java b/src/main/java/org/tron/walletcli/cli/aliases/AliasEntry.java
@@ -32,6 +32,9 @@ private AliasEntry(String name, AliasType type, byte[] address,
         if (source == null || source.trim().isEmpty()) {
             throw new IllegalArgumentException("source must not be blank");
         }
+        if (type == AliasType.TOKEN && (decimals < 0 || decimals > 18)) {
+            throw new IllegalArgumentException("token decimals must be between 0 and 18");
+        }
         this.name = n;
         this.type = type;
         this.address = address.clone();
diff --git a/src/main/java/org/tron/walletcli/cli/aliases/AliasStoreLoader.java b/src/main/java/org/tron/walletcli/cli/aliases/AliasStoreLoader.java
@@ -57,6 +57,22 @@ public AliasStore loadBuiltin(NetType network) throws IOException {
     }
 
     public AliasStore loadUser(NetType network) throws IOException {
+        try {
+            return loadUserOrThrow(network);
+        } catch (IOException e) {
+            warnFailed("read user alias file " + userFile(network).getPath(), e);
+            return AliasStore.empty();
+        } catch (RuntimeException e) {
+            warnFailed("read user alias file " + userFile(network).getPath(), e);
+            return AliasStore.empty();
+        }
+    }
+
+    /**
+     * Loads the user alias file and propagates read/parse failures. Mutating commands use this
+     * path so a malformed existing alias file is never treated as an empty store and overwritten.
+     */
+    public AliasStore loadUserOrThrow(NetType network) throws IOException {
         File file = userFile(network);
         if (!file.exists()) {
             return AliasStore.empty();
@@ -65,12 +81,6 @@ public AliasStore loadUser(NetType network) throws IOException {
         try {
             reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8);
             return readStore(reader, "user");
-        } catch (IOException e) {
-            warnFailed("read user alias file " + file.getPath(), e);
-            return AliasStore.empty();
-        } catch (RuntimeException e) {
-            warnFailed("read user alias file " + file.getPath(), e);
-            return AliasStore.empty();
         } finally {
             if (reader != null) {
                 closeQuietly(reader, "close user alias file " + file.getPath());
diff --git a/src/main/java/org/tron/walletcli/cli/commands/AliasCommands.java b/src/main/java/org/tron/walletcli/cli/commands/AliasCommands.java
@@ -68,7 +68,7 @@ private static void registerAdd(CommandRegistry registry) {
                                 + loader.networkName(network) + " and cannot be overridden", null);
                         return;
                     }
-                    AliasStore user = loader.loadUser(network);
+                    AliasStore user = loader.loadUserOrThrow(network);
                     if (user.containsName(name)) {
                         out.usageError("Alias already exists: " + name
                                 + ". Use alias-remove first to replace it.", null);
@@ -103,7 +103,7 @@ private static void registerRemove(CommandRegistry registry) {
                         return;
                     }
 
-                    AliasStore user = loader.loadUser(network);
+                    AliasStore user = loader.loadUserOrThrow(network);
                     List<AliasEntry> kept = new ArrayList<AliasEntry>();
                     boolean removed = false;
                     for (AliasEntry entry : user.listAll()) {
diff --git a/src/main/java/org/tron/walletcli/cli/ledger/LedgerPorts.java b/src/main/java/org/tron/walletcli/cli/ledger/LedgerPorts.java
@@ -50,10 +50,15 @@ public interface SignStateReader {
         void markSigning(String devicePath, String txid);
 
         /**
-         * Marks the current transaction as canceled after the standard CLI timeout path has
-         * stopped waiting for confirmation.
+         * Marks the current transaction as canceled or aborted so it does not remain signing.
          */
         void markCanceled(String devicePath, String txid);
+
+        /**
+         * Marks the current transaction as timed out after the standard CLI stops waiting for
+         * confirmation.
+         */
+        void markTimedOut(String devicePath, String txid);
     }
 
     /** Drives the actual APDU exchange and waits for the on-device button. */
diff --git a/src/main/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSigner.java b/src/main/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSigner.java
@@ -25,7 +25,8 @@ public final class NonInteractiveLedgerSigner implements LedgerSigner {
      */
     static final String STATE_SIGNING = "signing";    // SIGN_RESULT_SIGNING — in progress
     static final String STATE_CONFIRMED = "confirmed"; // SIGN_RESULT_SUCCESS — user confirmed
-    static final String STATE_CANCEL = "cancel";       // SIGN_RESULT_CANCEL — rejected or late response
+    static final String STATE_CANCEL = "cancel";       // SIGN_RESULT_CANCEL — user rejected/aborted
+    static final String STATE_TIMEOUT = "timeout";     // SIGN_RESULT_TIMEOUT — timed out
 
     /** APDU status word: Tron app is not open on the device. */
     private static final byte[] APDU_APP_IS_OPEN = new byte[] { 0x65, 0x11 };
@@ -102,13 +103,16 @@ public LedgerSignOutcome sign(Chain.Transaction transaction,
             byte[] apdu = executor.lastSendResultBytes();
             if (apdu != null && apdu.length > 0) {
                 if (matches(apdu, APDU_APP_IS_OPEN)) {
+                    stateReader.markCanceled(device.path(), txid);
                     return LedgerSignOutcome.failure(LedgerSignOutcome.Status.APP_NOT_OPEN,
                             "Open the Tron app on your Ledger device and try again");
                 }
                 if (matches(apdu, APDU_SIGN_BY_HASH)) {
+                    stateReader.markCanceled(device.path(), txid);
                     return LedgerSignOutcome.failure(LedgerSignOutcome.Status.SIGN_BY_HASH_DISABLED,
                             "Enable 'Sign By Hash' in the Ledger Tron app settings and try again");
                 }
+                stateReader.markCanceled(device.path(), txid);
                 return LedgerSignOutcome.failure(LedgerSignOutcome.Status.SIGN_FAILED,
                         "Ledger returned APDU error " + toHex(apdu));
             }
@@ -134,15 +138,16 @@ public LedgerSignOutcome sign(Chain.Transaction transaction,
                     return LedgerSignOutcome.failure(LedgerSignOutcome.Status.SIGN_FAILED,
                             "Ledger reported confirmation but no signature was recorded");
                 }
-                // STATE_CANCEL covers both the user-pressed-reject case and the rare
-                // "device responded after our 60s window" case. From the user's perspective
-                // both are "the sign did not complete because the user did not confirm in time".
                 if (STATE_CANCEL.equals(postState.get())) {
                     return LedgerSignOutcome.failure(LedgerSignOutcome.Status.USER_REJECTED,
                             "Transaction was rejected on the Ledger device");
                 }
+                if (STATE_TIMEOUT.equals(postState.get())) {
+                    return LedgerSignOutcome.failure(LedgerSignOutcome.Status.TIMEOUT,
+                            "Timed out waiting for confirmation on Ledger device");
+                }
                 if (STATE_SIGNING.equals(postState.get())) {
-                    stateReader.markCanceled(device.path(), txid);
+                    stateReader.markTimedOut(device.path(), txid);
                     return LedgerSignOutcome.failure(LedgerSignOutcome.Status.TIMEOUT,
                             "Timed out waiting for confirmation on Ledger device");
                 }
diff --git a/src/main/java/org/tron/walletcli/cli/ledger/ProductionLedgerPorts.java b/src/main/java/org/tron/walletcli/cli/ledger/ProductionLedgerPorts.java
@@ -67,6 +67,12 @@ public void markCanceled(String devicePath, String txid) {
                         LedgerSignResult.updateState(
                                 devicePath, txid, LedgerSignResult.SIGN_RESULT_CANCEL);
                     }
+
+                    @Override
+                    public void markTimedOut(String devicePath, String txid) {
+                        LedgerSignResult.updateState(
+                                devicePath, txid, LedgerSignResult.SIGN_RESULT_TIMEOUT);
+                    }
                 };
 
         LedgerPorts.SignExecutor executor = new LedgerPorts.SignExecutor() {
@@ -76,15 +82,20 @@ public boolean executeSignListen(LedgerPorts.DeviceHandle device, Chain.Transact
                 HidDevice raw = ((HidDeviceAdapter) device).delegate;
                 LedgerEventListener listener = LedgerEventListener.getInstance();
                 listener.setStandardCliQuiet(true);
-                listener.setLedgerSignEnd(new AtomicBoolean(false));
-                if (raw.isClosed()) {
-                    raw.open();
-                }
-                boolean accepted = listener.executeSignListen(raw, tx, path, gasfree);
-                if (listener.getLastSendResultBytes() != null || !accepted) {
+                try {
+                    listener.setLedgerSignEnd(new AtomicBoolean(false));
+                    if (raw.isClosed()) {
+                        raw.open();
+                    }
+                    boolean accepted = listener.executeSignListen(raw, tx, path, gasfree);
+                    if (listener.getLastSendResultBytes() != null || !accepted) {
+                        listener.setStandardCliQuiet(false);
+                    }
+                    return accepted;
+                } catch (RuntimeException e) {
                     listener.setStandardCliQuiet(false);
+                    throw e;
                 }
-                return accepted;
             }
 
             @Override
diff --git a/src/test/java/org/tron/walletcli/cli/aliases/AliasEntryTest.java b/src/test/java/org/tron/walletcli/cli/aliases/AliasEntryTest.java
@@ -37,4 +37,14 @@ public void addressIsCopiedDefensively() {
     public void rejectsWrongAddressLength() {
         AliasEntry.token("USDT", new byte[20], 6, "builtin");
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void rejectsNegativeTokenDecimals() {
+        AliasEntry.token("USDT", addr(), -1, "builtin");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void rejectsTooLargeTokenDecimals() {
+        AliasEntry.token("USDT", addr(), 19, "builtin");
+    }
 }
diff --git a/src/test/java/org/tron/walletcli/cli/aliases/AliasStoreLoaderTest.java b/src/test/java/org/tron/walletcli/cli/aliases/AliasStoreLoaderTest.java
@@ -13,6 +13,7 @@
 import org.tron.walletserver.WalletApi;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
@@ -92,6 +93,18 @@ public void malformedUserFileLoadsAsEmpty() throws Exception {
         assertEquals(0, store.listAll().size());
     }
 
+    @Test
+    public void malformedUserFileThrowsForStrictLoad() throws Exception {
+        writeUserFile(NetType.SHASTA, "{ broken json");
+
+        try {
+            loader.loadUserOrThrow(NetType.SHASTA);
+            fail("Expected malformed user alias file to fail strict load");
+        } catch (IOException e) {
+            assertNotNull(e.getMessage());
+        }
+    }
+
     @Test
     public void layeredLoadKeepsBuiltinsAuthoritative() throws Exception {
         writeUserFile(NetType.MAIN, "{ \"entries\": ["
diff --git a/src/test/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSignerTest.java b/src/test/java/org/tron/walletcli/cli/ledger/NonInteractiveLedgerSignerTest.java
@@ -121,6 +121,8 @@ public void returnsAppNotOpenOn0x6511() {
     executor.lastSendResult = new byte[]{0x65, 0x11};
     LedgerSignOutcome r = signNonGasfree();
     Assert.assertEquals(LedgerSignOutcome.Status.APP_NOT_OPEN, r.getStatus());
+    Assert.assertEquals("APDU failures must clear the current signing state",
+        NonInteractiveLedgerSigner.STATE_CANCEL, stateReader.updatedState);
   }
 
   @Test
@@ -129,6 +131,8 @@ public void returnsSignByHashDisabledOn0x6a8c() {
     executor.lastSendResult = new byte[]{0x6a, (byte) 0x8c};
     LedgerSignOutcome r = signNonGasfree();
     Assert.assertEquals(LedgerSignOutcome.Status.SIGN_BY_HASH_DISABLED, r.getStatus());
+    Assert.assertEquals("APDU failures must clear the current signing state",
+        NonInteractiveLedgerSigner.STATE_CANCEL, stateReader.updatedState);
   }
 
   @Test
@@ -138,6 +142,8 @@ public void returnsSignFailedOnUnknownApduResponse() {
     LedgerSignOutcome r = signNonGasfree();
     Assert.assertEquals(LedgerSignOutcome.Status.SIGN_FAILED, r.getStatus());
     Assert.assertTrue(r.getMessage().toLowerCase().contains("0xffff"));
+    Assert.assertEquals("APDU failures must clear the current signing state",
+        NonInteractiveLedgerSigner.STATE_CANCEL, stateReader.updatedState);
   }
 
   // ---------- pre-state ----------
@@ -164,6 +170,14 @@ public void returnsUserRejectedWhenPostStateIsCancel() {
     Assert.assertEquals(LedgerSignOutcome.Status.USER_REJECTED, r.getStatus());
   }
 
+  @Test
+  public void returnsTimeoutWhenPostStateIsTimeout() {
+    finder.next = new FakeDeviceHandle(DEV_PATH);
+    stateReader.postState = Optional.of(NonInteractiveLedgerSigner.STATE_TIMEOUT);
+    LedgerSignOutcome r = signNonGasfree();
+    Assert.assertEquals(LedgerSignOutcome.Status.TIMEOUT, r.getStatus());
+  }
+
   @Test
   public void returnsTimeoutWhenPostStateRemainsSigning() {
     finder.next = new FakeDeviceHandle(DEV_PATH);
@@ -172,9 +186,9 @@ public void returnsTimeoutWhenPostStateRemainsSigning() {
     Assert.assertEquals(LedgerSignOutcome.Status.TIMEOUT, r.getStatus());
     Assert.assertEquals("timeout must clear the current signing state so the next CLI command "
             + "is not blocked by a stale signing entry",
-        NonInteractiveLedgerSigner.STATE_CANCEL, stateReader.updatedState);
-    Assert.assertEquals(DEV_PATH, stateReader.updatedDevicePath);
-    Assert.assertNotNull(stateReader.updatedTxid);
+        NonInteractiveLedgerSigner.STATE_TIMEOUT, stateReader.timedOutState);
+    Assert.assertEquals(DEV_PATH, stateReader.timedOutDevicePath);
+    Assert.assertNotNull(stateReader.timedOutTxid);
   }
 
   @Test
@@ -291,6 +305,14 @@ private static final class FakeStateReader implements LedgerPorts.SignStateReade
       updatedTxid = txid;
       updatedState = NonInteractiveLedgerSigner.STATE_CANCEL;
     }
+    String timedOutDevicePath;
+    String timedOutTxid;
+    String timedOutState;
+    @Override public void markTimedOut(String devicePath, String txid) {
+      timedOutDevicePath = devicePath;
+      timedOutTxid = txid;
+      timedOutState = NonInteractiveLedgerSigner.STATE_TIMEOUT;
+    }
   }
 
   private static final class FakeExecutor implements LedgerPorts.SignExecutor {

Original file line number	Diff line number	Diff line change
`@@ -220,7 +220,7 @@ public void hidDataReceived(HidServicesEvent event) {`
`220`	`220`	`println("This transaction has expired, please resign and submit again.");`
`221`	`221`	`LedgerSignResult.updateState(`
`222`	`222`	`TransactionSignManager.getInstance().getHidDevice().getPath()`
`223`		`- , transactionId, LedgerSignResult.SIGN_RESULT_CANCEL`
	`223`	`+ , transactionId, LedgerSignResult.SIGN_RESULT_TIMEOUT`
`224`	`224`	`);`
`225`	`225`	`}`
`226`	`226`	`doLedgerSignEnd();`