From 2b5e22e2d4db8e5188312ca769ae00b2212490a9 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 28 Jul 2026 17:31:27 +0800 Subject: [PATCH 1/9] fix(core): validate TLS cert file and avoid interceptor leak in toString - withTLS(File) now rejects directories and unreadable files with a clear IllegalArgumentException at configuration time, instead of failing later in build() with a generic "Failed to create TLS channel" - toString() no longer expands customInterceptors (whose own toString may contain tokens or other sensitive data); it prints customInterceptorCount instead --- .../tron/trident/core/ApiWrapperBuilder.java | 6 ++- .../trident/core/ApiWrapperBuilderTest.java | 45 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java index f63f8e55..e5397490 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java @@ -58,6 +58,10 @@ public ApiWrapperBuilder withTLS(File certFile) { Preconditions.checkNotNull(certFile, "certFile is null"); Preconditions.checkArgument(certFile.exists(), "cert file does not exist: " + certFile.getAbsolutePath()); + Preconditions.checkArgument(certFile.isFile(), + "cert file is not a file: " + certFile.getAbsolutePath()); + Preconditions.checkArgument(certFile.canRead(), + "cert file is not readable: " + certFile.getAbsolutePath()); this.useTLS = true; this.trustCert = certFile; return this; @@ -172,7 +176,7 @@ public String toString() { .add("trustCert", trustCert != null ? trustCert.getAbsolutePath() : null) .add("apiKey", apiKey != null ? "****" : null) .add("timeoutMs", timeoutMs) - .add("customInterceptors", customInterceptors) + .add("customInterceptorCount", customInterceptors.size()) .toString(); } diff --git a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java index 631296a9..ce4a7840 100644 --- a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java +++ b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java @@ -157,6 +157,33 @@ void testWithTlsUsesSystemTrustCerts() { assertEquals(testCertFile, builder.getTrustCert()); } + @Test + void testWithTlsRejectsDirectory() throws IOException { + // a directory must be rejected immediately, not fail later in build() + File certDirectory = Files.createDirectory(tempDir.resolve("cert-dir")).toFile(); + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> { + new ApiWrapperBuilder(Constant.FULLNODE_NILE).withTLS(certDirectory); + }); + assertTrue(e.getMessage().contains("not a file")); + } + + @Test + void testWithTlsRejectsUnreadableFile() throws IOException { + File unreadable = tempDir.resolve("unreadable-cert.pem").toFile(); + Files.write(unreadable.toPath(), "dummy".getBytes()); + // skip silently if the platform does not support revoking read permission (e.g. root) + if (unreadable.setReadable(false) && !unreadable.canRead()) { + try { + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> { + new ApiWrapperBuilder(Constant.FULLNODE_NILE).withTLS(unreadable); + }); + assertTrue(e.getMessage().contains("not readable")); + } finally { + unreadable.setReadable(true); + } + } + } + @Test void testConstructorWithNullParameters() { // Test constructor with null parameters @@ -214,4 +241,22 @@ void testToString() { assertFalse(toStringResult.contains(TEST_API_KEY)); } + @Test + void testToStringDoesNotExposeInterceptorContent() { + String secret = "super-secret-token"; + ClientInterceptor leaky = new TimeoutInterceptor(1000L) { + @Override + public String toString() { + return "LeakyInterceptor{token=" + secret + "}"; + } + }; + String toStringResult = new ApiWrapperBuilder(Constant.FULLNODE_NILE) + .addInterceptors(Arrays.asList(leaky)) + .toString(); + + // interceptor instances must not be expanded, only their count is shown + assertTrue(toStringResult.contains("customInterceptorCount=1")); + assertFalse(toStringResult.contains(secret)); + } + } From cd96dc07d3d25e8edfac40ee3f66aef1c3036877 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 28 Jul 2026 17:59:10 +0800 Subject: [PATCH 2/9] fix(core): bound channel shutdown in ApiWrapper.close() close() previously only initiated an async shutdown and returned, so RPCs issued without a deadline could keep the channel alive indefinitely. Now it waits up to 5 seconds per channel for graceful termination, then falls back to shutdownNow(), restoring the interrupt flag if interrupted while waiting. --- .../java/org/tron/trident/core/ApiWrapper.java | 17 +++++++++++++++++ .../trident/core/ApiWrapperBuilderTest.java | 14 ++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index 5e4bbd61..69cf6201 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.TimeUnit; import lombok.Getter; import org.bouncycastle.jcajce.provider.digest.SHA256; import org.tron.trident.abi.FunctionEncoder; @@ -163,6 +164,7 @@ public class ApiWrapper implements Api { private static final String KEY_PAIR_NOT_SET = "keyPair is null, should set privateKey"; + private static final long CLOSE_TIMEOUT_SECONDS = 5; public final WalletGrpc.WalletBlockingStub blockingStub; public final WalletSolidityGrpc.WalletSolidityBlockingStub blockingStubSolidity; @@ -539,6 +541,21 @@ public void close() { if (channelSolidity != null) { channelSolidity.shutdown(); } + awaitTermination(channel); + if (channelSolidity != null) { + awaitTermination(channelSolidity); + } + } + + private void awaitTermination(ManagedChannel channel) { + try { + if (!channel.awaitTermination(CLOSE_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + channel.shutdownNow(); + } + } catch (InterruptedException e) { + channel.shutdownNow(); + Thread.currentThread().interrupt(); + } } @Override diff --git a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java index ce4a7840..aa8eab33 100644 --- a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java +++ b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java @@ -241,6 +241,20 @@ void testToString() { assertFalse(toStringResult.contains(TEST_API_KEY)); } + @Test + void testCloseTerminatesChannels() { + ApiWrapper wrapper = new ApiWrapperBuilder( + Constant.FULLNODE_NILE, + Constant.FULLNODE_NILE_SOLIDITY, + TEST_PRIVATE_KEY + ).build(); + + // close() must not return before both channels are fully terminated + wrapper.close(); + assertTrue(wrapper.channel.isTerminated()); + assertTrue(wrapper.channelSolidity.isTerminated()); + } + @Test void testToStringDoesNotExposeInterceptorContent() { String secret = "super-secret-token"; From 686a9f2b56a0580430c603c6961186c9291b7838 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 28 Jul 2026 18:07:09 +0800 Subject: [PATCH 3/9] fix(core): reject invalid length or prefix in parseAddress parseAddress previously returned whatever bytes the hex/base58 input decoded to, so inputs like "41", "0x41" or 22-byte hex strings produced malformed addresses that could even end up in SUCCESS transactions under local-create mode. The decoded bytes are now validated to be a proper TRON address (21 bytes with the 0x41 prefix) and rejected otherwise with a clear IllegalArgumentException. freezeBalance/unfreezeBalance treat the receiver as the optional field it is on chain: an empty receiveAddress (used by the no-receiver overloads, meaning the owner's own stake) skips setReceiverAddress instead of being passed through parseAddress, which would now reject it; non-empty receivers are still validated. --- .../org/tron/trident/core/ApiWrapper.java | 30 +++++++---- .../tron/trident/core/ParseAddressTest.java | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 core/src/test/java/org/tron/trident/core/ParseAddressTest.java diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index 69cf6201..06d4d92e 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -391,14 +391,18 @@ public static KeyPair generateAddress() { * * @param address account or contract address in any allowed formats. * @return hex address + * @throws IllegalArgumentException if the decoded address is not a valid TRON address + * (21 bytes with the 0x41 prefix) */ public static ByteString parseAddress(String address) { + Preconditions.checkArgument(!Strings.isEmpty(address), "address is null or empty"); byte[] raw; if (address.startsWith("T")) { raw = Base58Check.base58ToBytes(address); } else { raw = ByteArray.fromHexString(address); } + Preconditions.checkArgument(Utils.addressValid(raw), "invalid address: " + address); return ByteString.copyFrom(raw); } @@ -820,16 +824,17 @@ public TransactionExtention freezeBalance(String ownerAddress, long frozenBalanc @Override public TransactionExtention freezeBalance(String ownerAddress, long frozenBalance, int frozenDuration, int resourceCode, String receiveAddress) throws IllegalException { - ByteString rawFrom = parseAddress(ownerAddress); - ByteString rawReceiveFrom = parseAddress(receiveAddress); - FreezeBalanceContract freezeBalanceContract = + FreezeBalanceContract.Builder freezeBuilder = FreezeBalanceContract.newBuilder() - .setOwnerAddress(rawFrom) + .setOwnerAddress(parseAddress(ownerAddress)) .setFrozenBalance(frozenBalance) .setFrozenDuration(frozenDuration) - .setResourceValue(resourceCode) - .setReceiverAddress(rawReceiveFrom) - .build(); + .setResourceValue(resourceCode); + // the receiver is optional; an empty value means freezing for the owner itself + if (!Strings.isEmpty(receiveAddress)) { + freezeBuilder.setReceiverAddress(parseAddress(receiveAddress)); + } + FreezeBalanceContract freezeBalanceContract = freezeBuilder.build(); return createTransactionExtention(freezeBalanceContract, Transaction.Contract.ContractType.FreezeBalanceContract); } @@ -886,12 +891,15 @@ public TransactionExtention unfreezeBalance(String ownerAddress, int resourceCod public TransactionExtention unfreezeBalance(String ownerAddress, int resourceCode, String receiveAddress) throws IllegalException { - UnfreezeBalanceContract unfreezeBalanceContract = + UnfreezeBalanceContract.Builder unfreezeBuilder = UnfreezeBalanceContract.newBuilder() .setOwnerAddress(parseAddress(ownerAddress)) - .setResourceValue(resourceCode) - .setReceiverAddress(parseAddress(receiveAddress)) - .build(); + .setResourceValue(resourceCode); + // the receiver is optional; an empty value means unfreezing the owner's own stake + if (!Strings.isEmpty(receiveAddress)) { + unfreezeBuilder.setReceiverAddress(parseAddress(receiveAddress)); + } + UnfreezeBalanceContract unfreezeBalanceContract = unfreezeBuilder.build(); return createTransactionExtention(unfreezeBalanceContract, Transaction.Contract.ContractType.UnfreezeBalanceContract); diff --git a/core/src/test/java/org/tron/trident/core/ParseAddressTest.java b/core/src/test/java/org/tron/trident/core/ParseAddressTest.java new file mode 100644 index 00000000..3e2016e3 --- /dev/null +++ b/core/src/test/java/org/tron/trident/core/ParseAddressTest.java @@ -0,0 +1,53 @@ +package org.tron.trident.core; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.protobuf.ByteString; +import org.junit.jupiter.api.Test; +import org.tron.trident.core.key.KeyPair; +import org.tron.trident.core.utils.ByteArray; +import org.tron.trident.utils.Base58Check; + +/** + * Unit tests for ApiWrapper.parseAddress address validation + */ +class ParseAddressTest { + + private static final String VALID_BASE58 = KeyPair.generate().toBase58CheckAddress(); + private static final String VALID_HEX = + ByteArray.toHexString(Base58Check.base58ToBytes(VALID_BASE58)); + + @Test + void testParseValidAddresses() { + ByteString fromHex = ApiWrapper.parseAddress(VALID_HEX); + assertEquals(21, fromHex.size()); + assertEquals(VALID_HEX, ByteArray.toHexString(fromHex.toByteArray())); + + // "0x" prefix is accepted, and base58 decodes to the same bytes + assertEquals(fromHex, ApiWrapper.parseAddress("0x" + VALID_HEX)); + assertEquals(fromHex, ApiWrapper.parseAddress(VALID_BASE58)); + } + + @Test + void testParseRejectsInvalidLengthOrPrefix() { + // too short: a bare prefix byte is not an address + assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress("41")); + assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress("0x41")); + + // 22 bytes: one trailing byte too many + assertThrows(IllegalArgumentException.class, + () -> ApiWrapper.parseAddress(VALID_HEX + "00")); + + // 20 bytes: 0x41 prefix missing + assertThrows(IllegalArgumentException.class, + () -> ApiWrapper.parseAddress(VALID_HEX.substring(2))); + + // 21 bytes but wrong prefix byte + assertThrows(IllegalArgumentException.class, + () -> ApiWrapper.parseAddress("42" + VALID_HEX.substring(2))); + + assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress("")); + assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress(null)); + } +} From 879a4857ed58d5815106ded5411253e7531e4bd0 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 28 Jul 2026 18:23:56 +0800 Subject: [PATCH 4/9] fix(core): stop trusting caller-supplied txid in signTransaction signTransaction(TransactionExtention, KeyPair) previously signed whatever txid the extention carried, so a forged txid (e.g. transaction A's txid attached to transaction B) yielded a valid signature for a different transaction. The hash to sign is now always recomputed from the embedded transaction's raw data; a non-empty txid that does not match the raw data is rejected with IllegalArgumentException, and an absent txid no longer results in signing empty bytes. --- core/src/main/java/org/tron/trident/core/ApiWrapper.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index 06d4d92e..2eacc6ea 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -16,6 +16,7 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.TlsChannelCredentials; import java.io.IOException; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -565,7 +566,12 @@ private void awaitTermination(ManagedChannel channel) { @Override public Transaction signTransaction(TransactionExtention txnExt, KeyPair keyPair) { Preconditions.checkArgument(keyPair != null, "keyPair is null"); - byte[] txId = txnExt.getTxid().toByteArray(); + byte[] txId = calculateTransactionHash(txnExt.getTransaction()); + ByteString providedTxid = txnExt.getTxid(); + if (!providedTxid.isEmpty()) { + Preconditions.checkArgument(Arrays.equals(txId, providedTxid.toByteArray()), + "txid does not match the transaction raw data"); + } byte[] signature = KeyPair.signTransaction(txId, keyPair); return txnExt.getTransaction().toBuilder().addSignature(ByteString.copyFrom(signature)).build(); } From 5d69d6ce3c219a424ff84c72fc5c209ac43dec8a Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Tue, 28 Jul 2026 18:56:49 +0800 Subject: [PATCH 5/9] fix(utils,core): reject secp256k1 private keys outside [1, n - 1] SECP256K1.PrivateKey.create previously accepted any 32-byte scalar, so 0, the curve order n, and n + 1 all produced usable-looking KeyPairs that could derive addresses (n + 1 even deriving the same address as private key 1 via the mod-n reduction in PublicKey.create) but failed later at signing time inside Bouncy Castle. The scalar range is now validated at construction with the same [1, n - 1] rule Bouncy Castle enforces, covering KeyPair(String) and all other construction paths. ApiWrapperBuilder.withPrivateKey performs the same check up front so invalid keys fail at input time rather than build(). --- .../tron/trident/core/ApiWrapperBuilder.java | 7 ++++ .../trident/core/ApiWrapperBuilderTest.java | 19 +++++++++ .../org/tron/trident/crypto/SECP256K1.java | 10 +++++ .../crypto/SECP256K1PrivateKeyTest.java | 40 +++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 utils/src/test/java/org/tron/trident/crypto/SECP256K1PrivateKeyTest.java diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java index e5397490..ea62f79d 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java @@ -11,6 +11,7 @@ import java.util.List; import lombok.Getter; import org.tron.trident.core.interceptor.TimeoutInterceptor; +import org.tron.trident.crypto.SECP256K1; import org.tron.trident.utils.Numeric; import org.tron.trident.utils.Strings; @@ -126,11 +127,17 @@ public ApiWrapperBuilder withGrpcEndpointSolidity(String grpcEndpointSolidity) { /** * set PrivateKey, an optional "0x" prefix is accepted + * + * @throws IllegalArgumentException if the key is not 64 hex characters or its scalar + * is outside the valid secp256k1 range [1, n - 1] */ public ApiWrapperBuilder withPrivateKey(String hexPrivateKey) { String cleaned = Numeric.cleanHexPrefix(hexPrivateKey); Preconditions.checkArgument(cleaned != null && cleaned.length() == 64, "hexPrivateKey should be 64 hex characters (32 bytes)"); + // fail fast here instead of at build(): rejects scalars outside [1, n - 1] + // and non-hex characters + SECP256K1.PrivateKey.create(cleaned); this.hexPrivateKey = cleaned; return this; } diff --git a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java index aa8eab33..21fbd6fd 100644 --- a/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java +++ b/core/src/test/java/org/tron/trident/core/ApiWrapperBuilderTest.java @@ -10,6 +10,7 @@ import io.grpc.ClientInterceptor; import java.io.File; import java.io.IOException; +import java.math.BigInteger; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; @@ -19,6 +20,7 @@ import org.junit.jupiter.api.io.TempDir; import org.tron.trident.core.interceptor.TimeoutInterceptor; import org.tron.trident.core.key.KeyPair; +import org.tron.trident.crypto.SECP256K1; /** * Unit tests for ApiWrapperBuilder class @@ -109,6 +111,23 @@ void testPrivateKeyAcceptsHexPrefix() { }); } + @Test + void testWithPrivateKeyRejectsOutOfRangeScalars() { + BigInteger n = SECP256K1.CURVE.getN(); + for (BigInteger invalid : new BigInteger[] { + BigInteger.ZERO, n, n.add(BigInteger.ONE)}) { + assertThrows(IllegalArgumentException.class, () -> { + new ApiWrapperBuilder(Constant.FULLNODE_NILE) + .withPrivateKey(String.format("%064x", invalid)); + }); + } + + // n - 1 is the top of the valid range and must still be accepted + String nMinus1 = String.format("%064x", n.subtract(BigInteger.ONE)); + assertEquals(nMinus1, new ApiWrapperBuilder(Constant.FULLNODE_NILE) + .withPrivateKey(nMinus1).getHexPrivateKey()); + } + @Test void testRepeatedSettersLastWins() { ApiWrapperBuilder builder = new ApiWrapperBuilder(Constant.FULLNODE_NILE) diff --git a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java index a1bd4fc9..e8ac3254 100644 --- a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java +++ b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java @@ -324,6 +324,16 @@ public static PrivateKey create(final BigInteger key) { } public static PrivateKey create(final Bytes32 key) { + Preconditions.checkNotNull(key, "key must not be null"); + // Reject out-of-range scalars at construction instead of letting Bouncy Castle + // throw the same error later at signing time (ECDomainParameters + // .validatePrivateScalar). Without this, 0 and n fail only when signing, and + // n + 1 even derives the same address as private key 1 via the mod-n reduction + // in PublicKey.create. + final BigInteger d = key.toUnsignedBigInteger(); + Preconditions.checkArgument( + d.compareTo(BigInteger.ONE) >= 0 && d.compareTo(CURVE_ORDER) < 0, + "Scalar is not in the interval [1, n - 1]"); return new PrivateKey(key); } diff --git a/utils/src/test/java/org/tron/trident/crypto/SECP256K1PrivateKeyTest.java b/utils/src/test/java/org/tron/trident/crypto/SECP256K1PrivateKeyTest.java new file mode 100644 index 00000000..5042d57f --- /dev/null +++ b/utils/src/test/java/org/tron/trident/crypto/SECP256K1PrivateKeyTest.java @@ -0,0 +1,40 @@ +package org.tron.trident.crypto; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.math.BigInteger; +import org.junit.jupiter.api.Test; +import org.tron.trident.crypto.tuwenitypes.Bytes32; +import org.tron.trident.crypto.tuwenitypes.UInt256; + +/** + * Unit tests for the secp256k1 private key scalar range check + */ +class SECP256K1PrivateKeyTest { + + private static final BigInteger N = SECP256K1.CURVE.getN(); + + private static Bytes32 scalar(BigInteger value) { + return UInt256.valueOf(value).toBytes(); + } + + @Test + void testCreateRejectsOutOfRangeScalars() { + for (BigInteger invalid : new BigInteger[] {BigInteger.ZERO, N, N.add(BigInteger.ONE)}) { + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> SECP256K1.PrivateKey.create(scalar(invalid))); + assertTrue(e.getMessage().contains("[1, n - 1]")); + } + } + + @Test + void testCreateAcceptsRangeBoundaries() { + // 1 and n - 1 are the inclusive bounds of the valid range + assertNotNull(SECP256K1.PrivateKey.create(scalar(BigInteger.ONE))); + assertNotNull(SECP256K1.PrivateKey.create(scalar(N.subtract(BigInteger.ONE)))); + // generated keys pass their own validation + assertNotNull(SECP256K1.KeyPair.generate()); + } +} From 6f97eecd2296e6a3133c687ab25acd75e1ba9061 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 29 Jul 2026 11:10:41 +0800 Subject: [PATCH 6/9] fix(core): restore empty-address semantics and reject incomplete signing parseAddress maps an empty string to ByteString.EMPTY (an unset protobuf field, the pre-1.0 behavior for optional addresses such as a freeze receiver or a constant-call owner), rejects input longer than 64 characters before decoding, and rethrows decoding failures as the documented IllegalArgumentException. The freeze/unfreeze receiver guards are reverted accordingly. signTransaction now rejects empty transaction raw data (previously signed silently, swallowing node errors) and, for TransactionExtention, requires a txid that is present and matches the raw data. --- .../org/tron/trident/core/ApiWrapper.java | 66 +++++++++++-------- .../tron/trident/core/ParseAddressTest.java | 25 ++++++- 2 files changed, 63 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index 2eacc6ea..09e15e40 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -166,6 +166,9 @@ public class ApiWrapper implements Api { private static final String KEY_PAIR_NOT_SET = "keyPair is null, should set privateKey"; private static final long CLOSE_TIMEOUT_SECONDS = 5; + // upper bound for any supported address form (base58: 34, hex: 42, 0x-hex: 44); + // rejects oversized input before decoding to keep hostile strings cheap + private static final int MAX_ADDRESS_LENGTH = 64; public final WalletGrpc.WalletBlockingStub blockingStub; public final WalletSolidityGrpc.WalletSolidityBlockingStub blockingStubSolidity; @@ -390,18 +393,28 @@ public static KeyPair generateAddress() { /** * The function receives addresses in any formats. * - * @param address account or contract address in any allowed formats. + * @param address account or contract address in any allowed formats. An empty string + * returns {@link ByteString#EMPTY}, which leaves the protobuf address field unset. * @return hex address * @throws IllegalArgumentException if the decoded address is not a valid TRON address - * (21 bytes with the 0x41 prefix) */ public static ByteString parseAddress(String address) { - Preconditions.checkArgument(!Strings.isEmpty(address), "address is null or empty"); + Preconditions.checkNotNull(address, "address is null"); + Preconditions.checkArgument(address.length() <= MAX_ADDRESS_LENGTH, + "invalid address length: " + address.length()); + if (address.isEmpty()) { + return ByteString.EMPTY; + } byte[] raw; - if (address.startsWith("T")) { - raw = Base58Check.base58ToBytes(address); - } else { - raw = ByteArray.fromHexString(address); + try { + if (address.startsWith("T")) { + raw = Base58Check.base58ToBytes(address); + } else { + raw = ByteArray.fromHexString(address); + } + } catch (Exception e) { + throw new IllegalArgumentException( + "invalid address: " + address + " (" + e.getMessage() + ")"); } Preconditions.checkArgument(Utils.addressValid(raw), "invalid address: " + address); return ByteString.copyFrom(raw); @@ -566,12 +579,17 @@ private void awaitTermination(ManagedChannel channel) { @Override public Transaction signTransaction(TransactionExtention txnExt, KeyPair keyPair) { Preconditions.checkArgument(keyPair != null, "keyPair is null"); - byte[] txId = calculateTransactionHash(txnExt.getTransaction()); - ByteString providedTxid = txnExt.getTxid(); - if (!providedTxid.isEmpty()) { - Preconditions.checkArgument(Arrays.equals(txId, providedTxid.toByteArray()), - "txid does not match the transaction raw data"); + if (txnExt.getTransaction().getRawData().getSerializedSize() == 0) { + String detail = txnExt.getResult().getMessage().toStringUtf8(); + throw new IllegalArgumentException( + "txnExt carries no transaction" + (detail.isEmpty() ? "" : ": " + detail)); } + ByteString providedTxid = txnExt.getTxid(); + Preconditions.checkArgument(!providedTxid.isEmpty(), + "txnExt has no txid; use signTransaction(Transaction, KeyPair) for a raw transaction"); + byte[] txId = calculateTransactionHash(txnExt.getTransaction()); + Preconditions.checkArgument(Arrays.equals(txId, providedTxid.toByteArray()), + "txid does not match the transaction raw data"); byte[] signature = KeyPair.signTransaction(txId, keyPair); return txnExt.getTransaction().toBuilder().addSignature(ByteString.copyFrom(signature)).build(); } @@ -579,6 +597,8 @@ public Transaction signTransaction(TransactionExtention txnExt, KeyPair keyPair) @Override public Transaction signTransaction(Transaction txn, KeyPair keyPair) { Preconditions.checkArgument(keyPair != null, "keyPair is null"); + Preconditions.checkArgument(txn.getRawData().getSerializedSize() > 0, + "transaction raw data is empty"); byte[] txId = calculateTransactionHash(txn); byte[] signature = KeyPair.signTransaction(txId, keyPair); return txn.toBuilder().addSignature(ByteString.copyFrom(signature)).build(); @@ -830,17 +850,14 @@ public TransactionExtention freezeBalance(String ownerAddress, long frozenBalanc @Override public TransactionExtention freezeBalance(String ownerAddress, long frozenBalance, int frozenDuration, int resourceCode, String receiveAddress) throws IllegalException { - FreezeBalanceContract.Builder freezeBuilder = + FreezeBalanceContract freezeBalanceContract = FreezeBalanceContract.newBuilder() .setOwnerAddress(parseAddress(ownerAddress)) .setFrozenBalance(frozenBalance) .setFrozenDuration(frozenDuration) - .setResourceValue(resourceCode); - // the receiver is optional; an empty value means freezing for the owner itself - if (!Strings.isEmpty(receiveAddress)) { - freezeBuilder.setReceiverAddress(parseAddress(receiveAddress)); - } - FreezeBalanceContract freezeBalanceContract = freezeBuilder.build(); + .setResourceValue(resourceCode) + .setReceiverAddress(parseAddress(receiveAddress)) + .build(); return createTransactionExtention(freezeBalanceContract, Transaction.Contract.ContractType.FreezeBalanceContract); } @@ -897,15 +914,12 @@ public TransactionExtention unfreezeBalance(String ownerAddress, int resourceCod public TransactionExtention unfreezeBalance(String ownerAddress, int resourceCode, String receiveAddress) throws IllegalException { - UnfreezeBalanceContract.Builder unfreezeBuilder = + UnfreezeBalanceContract unfreezeBalanceContract = UnfreezeBalanceContract.newBuilder() .setOwnerAddress(parseAddress(ownerAddress)) - .setResourceValue(resourceCode); - // the receiver is optional; an empty value means unfreezing the owner's own stake - if (!Strings.isEmpty(receiveAddress)) { - unfreezeBuilder.setReceiverAddress(parseAddress(receiveAddress)); - } - UnfreezeBalanceContract unfreezeBalanceContract = unfreezeBuilder.build(); + .setResourceValue(resourceCode) + .setReceiverAddress(parseAddress(receiveAddress)) + .build(); return createTransactionExtention(unfreezeBalanceContract, Transaction.Contract.ContractType.UnfreezeBalanceContract); diff --git a/core/src/test/java/org/tron/trident/core/ParseAddressTest.java b/core/src/test/java/org/tron/trident/core/ParseAddressTest.java index 3e2016e3..e65dba62 100644 --- a/core/src/test/java/org/tron/trident/core/ParseAddressTest.java +++ b/core/src/test/java/org/tron/trident/core/ParseAddressTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.protobuf.ByteString; import org.junit.jupiter.api.Test; @@ -47,7 +48,27 @@ void testParseRejectsInvalidLengthOrPrefix() { assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress("42" + VALID_HEX.substring(2))); - assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress("")); - assertThrows(IllegalArgumentException.class, () -> ApiWrapper.parseAddress(null)); + assertThrows(NullPointerException.class, () -> ApiWrapper.parseAddress(null)); + + // undecodable input surfaces as IllegalArgumentException per the javadoc, + // not as Bouncy Castle's DecoderException (an IllegalStateException) + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> ApiWrapper.parseAddress("zz" + VALID_HEX.substring(2))); + assertTrue(e.getMessage().contains("invalid address")); + + // oversized input is rejected before decoding, echoing only the length + StringBuilder oversized = new StringBuilder(VALID_HEX); + for (int i = 0; i < 1000; i++) { + oversized.append("00"); + } + IllegalArgumentException tooLong = assertThrows(IllegalArgumentException.class, + () -> ApiWrapper.parseAddress(oversized.toString())); + assertTrue(tooLong.getMessage().contains("invalid address length")); + } + + @Test + void testParseEmptyMeansUnsetOptionalField() { + // "" maps to an unset optional protobuf field (freeze receiver, constant-call owner) + assertEquals(ByteString.EMPTY, ApiWrapper.parseAddress("")); } } From 96a347842c27f51f1188d2836ca71f17ed1dc411 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 29 Jul 2026 12:23:41 +0800 Subject: [PATCH 7/9] fix(utils,core): stop replacing the JVM-global Bouncy Castle provider Loading SECP256K1 used to remove the host's "BC" provider and insert the SDK's own instance at priority 1, silently changing algorithm resolution and compliance posture for the whole process. The provider is now passed explicitly to KeyPairGenerator and the JVM-global provider list is no longer touched; the bundled full provider still covers Android's stripped "BC". Applications that implicitly relied on trident registering BC globally must now register it themselves. The hand-written [1, n - 1] scalar check is replaced with Bouncy Castle's own ECDomainParameters.validatePrivateScalar, and withPrivateKey wraps validation failures as "invalid hexPrivateKey (reason)" without echoing key material. --- .../tron/trident/core/ApiWrapperBuilder.java | 8 +++++-- .../org/tron/trident/crypto/SECP256K1.java | 21 +++++++------------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java index ea62f79d..c1cc7cf7 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapperBuilder.java @@ -136,8 +136,12 @@ public ApiWrapperBuilder withPrivateKey(String hexPrivateKey) { Preconditions.checkArgument(cleaned != null && cleaned.length() == 64, "hexPrivateKey should be 64 hex characters (32 bytes)"); // fail fast here instead of at build(): rejects scalars outside [1, n - 1] - // and non-hex characters - SECP256K1.PrivateKey.create(cleaned); + // and non-hex characters; never echo the key material in the message + try { + SECP256K1.PrivateKey.create(cleaned); + } catch (Exception e) { + throw new IllegalArgumentException("invalid hexPrivateKey (" + e.getMessage() + ")"); + } this.hexPrivateKey = cleaned; return this; } diff --git a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java index e8ac3254..928a0e49 100644 --- a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java +++ b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java @@ -19,7 +19,6 @@ import java.math.BigInteger; import java.security.InvalidAlgorithmParameterException; import java.security.KeyPairGenerator; -import java.security.Security; import java.security.spec.ECGenParameterSpec; import java.util.Arrays; import java.util.Objects; @@ -68,16 +67,14 @@ public class SECP256K1 { private static final BigInteger CURVE_ORDER; static { - //support android platform - Security.removeProvider(BouncyCastleProvider.PROVIDER_NAME); - Security.insertProviderAt(new BouncyCastleProvider(), 1); - final X9ECParameters params = SECNamedCurves.getByName(CURVE_NAME); CURVE = new ECDomainParameters(params.getCurve(), params.getG(), params.getN(), params.getH()); CURVE_ORDER = CURVE.getN(); HALF_CURVE_ORDER = CURVE_ORDER.shiftRight(1); try { - KEY_PAIR_GENERATOR = KeyPairGenerator.getInstance(ALGORITHM, PROVIDER); + // pass the provider explicitly instead of touching the JVM-global provider + // list; also bypasses Android's stripped built-in "BC" + KEY_PAIR_GENERATOR = KeyPairGenerator.getInstance(ALGORITHM, new BouncyCastleProvider()); } catch (final Exception e) { throw new RuntimeException(e); } @@ -326,14 +323,10 @@ public static PrivateKey create(final BigInteger key) { public static PrivateKey create(final Bytes32 key) { Preconditions.checkNotNull(key, "key must not be null"); // Reject out-of-range scalars at construction instead of letting Bouncy Castle - // throw the same error later at signing time (ECDomainParameters - // .validatePrivateScalar). Without this, 0 and n fail only when signing, and - // n + 1 even derives the same address as private key 1 via the mod-n reduction - // in PublicKey.create. - final BigInteger d = key.toUnsignedBigInteger(); - Preconditions.checkArgument( - d.compareTo(BigInteger.ONE) >= 0 && d.compareTo(CURVE_ORDER) < 0, - "Scalar is not in the interval [1, n - 1]"); + // throw the same error later at signing time. Without this, 0 and n fail only + // when signing, and n + 1 even derives the same address as private key 1 via + // the mod-n reduction in PublicKey.create. + CURVE.validatePrivateScalar(key.toUnsignedBigInteger()); return new PrivateKey(key); } From 637fa37023f0dfed7663895a648d3833e520c475 Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Wed, 29 Jul 2026 14:33:17 +0800 Subject: [PATCH 8/9] fix(utils,core): deprecate SECP256K1.PROVIDER, tighten address bound, complete close() - deprecate SECP256K1.PROVIDER: trident no longer registers Bouncy Castle in the JVM-global provider list, so a provider named "BC" is not guaranteed to exist; callers that need it must register it themselves - tighten parseAddress MAX_ADDRESS_LENGTH from 64 to 44, the largest legal address form (base58: 34, hex: 42, 0x-hex: 44), so a 64-char hex private key passed by mistake is rejected by the length gate instead of being echoed into the exception message - close(): awaitTermination() called shutdownNow() on timeout and returned immediately, so close() could return with the channel still not terminated; it now waits again for the forced cancellation to take effect --- core/src/main/java/org/tron/trident/core/ApiWrapper.java | 4 ++-- utils/src/main/java/org/tron/trident/crypto/SECP256K1.java | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index 09e15e40..fc908ea8 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -167,8 +167,7 @@ public class ApiWrapper implements Api { private static final String KEY_PAIR_NOT_SET = "keyPair is null, should set privateKey"; private static final long CLOSE_TIMEOUT_SECONDS = 5; // upper bound for any supported address form (base58: 34, hex: 42, 0x-hex: 44); - // rejects oversized input before decoding to keep hostile strings cheap - private static final int MAX_ADDRESS_LENGTH = 64; + private static final int MAX_ADDRESS_LENGTH = 44; public final WalletGrpc.WalletBlockingStub blockingStub; public final WalletSolidityGrpc.WalletSolidityBlockingStub blockingStubSolidity; @@ -569,6 +568,7 @@ private void awaitTermination(ManagedChannel channel) { try { if (!channel.awaitTermination(CLOSE_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { channel.shutdownNow(); + channel.awaitTermination(CLOSE_TIMEOUT_SECONDS, TimeUnit.SECONDS); } } catch (InterruptedException e) { channel.shutdownNow(); diff --git a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java index 928a0e49..d8c5ece0 100644 --- a/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java +++ b/utils/src/main/java/org/tron/trident/crypto/SECP256K1.java @@ -58,6 +58,13 @@ public class SECP256K1 { public static final String ALGORITHM = "ECDSA"; public static final String CURVE_NAME = "secp256k1"; + /** + * @deprecated trident no longer registers Bouncy Castle in the JVM-global + * provider list, so a provider named "BC" is not guaranteed to exist. + * Callers that need it must register it themselves, e.g. + * {@code Security.addProvider(new BouncyCastleProvider())}. + */ + @Deprecated public static final String PROVIDER = "BC"; public static final ECDomainParameters CURVE; From 0a00e5e0aa07357b56cafa9a7f399d56372b3bab Mon Sep 17 00:00:00 2001 From: 0xbigapple Date: Thu, 30 Jul 2026 13:44:20 +0800 Subject: [PATCH 9/9] modify comments --- core/src/main/java/org/tron/trident/core/ApiWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/tron/trident/core/ApiWrapper.java b/core/src/main/java/org/tron/trident/core/ApiWrapper.java index fc908ea8..20386979 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -586,7 +586,7 @@ public Transaction signTransaction(TransactionExtention txnExt, KeyPair keyPair) } ByteString providedTxid = txnExt.getTxid(); Preconditions.checkArgument(!providedTxid.isEmpty(), - "txnExt has no txid; use signTransaction(Transaction, KeyPair) for a raw transaction"); + "txnExt has no txid"); byte[] txId = calculateTransactionHash(txnExt.getTransaction()); Preconditions.checkArgument(Arrays.equals(txId, providedTxid.toByteArray()), "txid does not match the transaction raw data");