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..20386979 100644 --- a/core/src/main/java/org/tron/trident/core/ApiWrapper.java +++ b/core/src/main/java/org/tron/trident/core/ApiWrapper.java @@ -16,10 +16,12 @@ 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; 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 +165,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); + private static final int MAX_ADDRESS_LENGTH = 44; public final WalletGrpc.WalletBlockingStub blockingStub; public final WalletSolidityGrpc.WalletSolidityBlockingStub blockingStubSolidity; @@ -387,16 +392,30 @@ 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 */ public static ByteString parseAddress(String address) { + 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); } @@ -539,12 +558,38 @@ 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(); + channel.awaitTermination(CLOSE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } + } catch (InterruptedException e) { + channel.shutdownNow(); + Thread.currentThread().interrupt(); + } } @Override public Transaction signTransaction(TransactionExtention txnExt, KeyPair keyPair) { Preconditions.checkArgument(keyPair != null, "keyPair is null"); - byte[] txId = txnExt.getTxid().toByteArray(); + 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"); + 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(); } @@ -552,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(); @@ -803,15 +850,13 @@ 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.newBuilder() - .setOwnerAddress(rawFrom) + .setOwnerAddress(parseAddress(ownerAddress)) .setFrozenBalance(frozenBalance) .setFrozenDuration(frozenDuration) .setResourceValue(resourceCode) - .setReceiverAddress(rawReceiveFrom) + .setReceiverAddress(parseAddress(receiveAddress)) .build(); return createTransactionExtention(freezeBalanceContract, Transaction.Contract.ContractType.FreezeBalanceContract); 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..c1cc7cf7 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; @@ -58,6 +59,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; @@ -122,11 +127,21 @@ 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; 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; } @@ -172,7 +187,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..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) @@ -157,6 +176,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 +260,36 @@ 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"; + 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)); + } + } 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..e65dba62 --- /dev/null +++ b/core/src/test/java/org/tron/trident/core/ParseAddressTest.java @@ -0,0 +1,74 @@ +package org.tron.trident.core; + +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; +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(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("")); + } +} 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..d8c5ece0 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; @@ -59,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; @@ -68,16 +74,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); } @@ -324,6 +328,12 @@ 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. 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); } 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()); + } +}