diff --git a/common/src/main/java/org/tron/core/Constant.java b/common/src/main/java/org/tron/core/Constant.java index 1437d319346..89d4c31e9d2 100644 --- a/common/src/main/java/org/tron/core/Constant.java +++ b/common/src/main/java/org/tron/core/Constant.java @@ -19,7 +19,8 @@ public class Constant { public static final long MAXIMUM_TIME_UNTIL_EXPIRATION = 24 * 60 * 60 * 1_000L; //one day public static final long TRANSACTION_DEFAULT_EXPIRATION_TIME = 60 * 1_000L; //60 seconds public static final long TRANSACTION_FEE_POOL_PERIOD = 1; //1 blocks - public static final long PER_SIGN_LENGTH = 65L; + public static final int PER_SIGN_LENGTH = 65; + public static final int MAX_PER_SIGN_LENGTH = 68; public static final long MAX_CONTRACT_RESULT_SIZE = 2L; // Smart contract / Energy diff --git a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java index b921d548e8b..e0e20fb2677 100644 --- a/crypto/src/main/java/org/tron/common/crypto/SignUtils.java +++ b/crypto/src/main/java/org/tron/common/crypto/SignUtils.java @@ -1,5 +1,8 @@ package org.tron.common.crypto; +import static org.tron.core.Constant.MAX_PER_SIGN_LENGTH; +import static org.tron.core.Constant.PER_SIGN_LENGTH; + import java.security.SecureRandom; import java.security.SignatureException; import org.tron.common.crypto.ECKey.ECDSASignature; @@ -8,6 +11,21 @@ public class SignUtils { + /** + * Strict signature-length check for admission entry-points (RPC broadcast, + * P2P transaction ingress, peer hello handshake). Accepts only sizes in + * [{@link org.tron.core.Constant#PER_SIGN_LENGTH PER_SIGN_LENGTH}, + * {@link org.tron.core.Constant#MAX_PER_SIGN_LENGTH MAX_PER_SIGN_LENGTH}]. + * + *

Consensus paths (e.g. {@code TransactionCapsule.checkWeight}) intentionally + * keep the looser {@code size < 65} check to remain compatible with historical + * on-chain signatures that carry trailing padding bytes; do not call this + * helper from those paths. + */ + public static boolean isValidLength(int size) { + return size >= PER_SIGN_LENGTH && size <= MAX_PER_SIGN_LENGTH; + } + public static SignInterface getGeneratedRandomSign( SecureRandom secureRandom, boolean isECKeyCryptoEngine) { if (isECKeyCryptoEngine) { diff --git a/framework/src/main/java/org/tron/core/Wallet.java b/framework/src/main/java/org/tron/core/Wallet.java index 0482643d8d0..393a3cb8854 100755 --- a/framework/src/main/java/org/tron/core/Wallet.java +++ b/framework/src/main/java/org/tron/core/Wallet.java @@ -505,6 +505,16 @@ public GrpcAPI.Return broadcastTransaction(Transaction signedTransaction) { trx.setTime(System.currentTimeMillis()); Sha256Hash txID = trx.getTransactionId(); try { + for (ByteString sig : signedTransaction.getSignatureList()) { + if (!SignUtils.isValidLength(sig.size())) { + String info = "Signature size is " + sig.size(); + logger.warn("Broadcast transaction {} has failed, {}.", txID, info); + return builder.setResult(false).setCode(response_code.SIGERROR) + .setMessage(ByteString.copyFromUtf8("Validate signature error: " + info)) + .build(); + } + } + if (tronNetDelegate.isBlockUnsolidified()) { logger.warn("Broadcast transaction {} has failed, block unsolidified.", txID); return builder.setResult(false).setCode(response_code.BLOCK_UNSOLIDIFIED) diff --git a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java index e153e21f331..52137c5881c 100644 --- a/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java +++ b/framework/src/main/java/org/tron/core/net/messagehandler/TransactionsMsgHandler.java @@ -1,5 +1,6 @@ package org.tron.core.net.messagehandler; +import com.google.protobuf.ByteString; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -13,6 +14,7 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import org.tron.common.crypto.SignUtils; import org.tron.common.es.ExecutorServiceManager; import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; @@ -142,6 +144,12 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep throw new P2pException(TypeEnum.BAD_TRX, "tx " + item.getHash() + " contract size should be greater than 0"); } + for (ByteString sig : trx.getSignatureList()) { + if (!SignUtils.isValidLength(sig.size())) { + throw new P2pException(TypeEnum.BAD_TRX, + "tx " + item.getHash() + " signature size is " + sig.size()); + } + } } } diff --git a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java index 61ae6326e9f..d4e010ff21d 100644 --- a/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java +++ b/framework/src/main/java/org/tron/core/net/service/relay/RelayService.java @@ -150,6 +150,12 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) { return false; } + if (!SignUtils.isValidLength(msg.getSignature().size())) { + logger.warn("HelloMessage from {}, signature size is {}.", + channel.getInetAddress(), msg.getSignature().size()); + return false; + } + boolean flag; try { Sha256Hash hash = Sha256Hash.of(CommonParameter diff --git a/framework/src/test/java/org/tron/core/WalletMockTest.java b/framework/src/test/java/org/tron/core/WalletMockTest.java index 3e0c1a4461d..c9184bf276d 100644 --- a/framework/src/test/java/org/tron/core/WalletMockTest.java +++ b/framework/src/test/java/org/tron/core/WalletMockTest.java @@ -164,6 +164,54 @@ public void testCreateTransactionCapsuleWithoutValidateWithTimeout() } + @Test + public void testBroadcastTxInvalidSigLength() throws Exception { + Wallet wallet = new Wallet(); + TronNetDelegate tronNetDelegateMock = mock(TronNetDelegate.class); + Field field = wallet.getClass().getDeclaredField("tronNetDelegate"); + field.setAccessible(true); + field.set(wallet, tronNetDelegateMock); + + // signature shorter than 65 bytes → SIGERROR + Protocol.Transaction shortSig = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[64])) + .build(); + GrpcAPI.Return ret = wallet.broadcastTransaction(shortSig); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + + // signature longer than 68 bytes → SIGERROR + Protocol.Transaction longSig = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[69])) + .build(); + ret = wallet.broadcastTransaction(longSig); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + + // empty signature → SIGERROR + Protocol.Transaction emptySig = Protocol.Transaction.newBuilder() + .addSignature(ByteString.EMPTY) + .build(); + ret = wallet.broadcastTransaction(emptySig); + assertEquals(GrpcAPI.Return.response_code.SIGERROR, ret.getCode()); + + // tronNetDelegate must not be consulted because the request is rejected up front + Mockito.verify(tronNetDelegateMock, Mockito.never()).isBlockUnsolidified(); + + // 65-byte signature passes the length check and proceeds to downstream logic + when(tronNetDelegateMock.isBlockUnsolidified()).thenReturn(true); + Protocol.Transaction validSig = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[65])) + .build(); + ret = wallet.broadcastTransaction(validSig); + assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); + + // 68-byte signature (upper bound) also passes the length check + Protocol.Transaction paddedSig = Protocol.Transaction.newBuilder() + .addSignature(ByteString.copyFrom(new byte[68])) + .build(); + ret = wallet.broadcastTransaction(paddedSig); + assertEquals(GrpcAPI.Return.response_code.BLOCK_UNSOLIDIFIED, ret.getCode()); + } + @Test public void testBroadcastTransactionBlockUnsolidified() throws Exception { Wallet wallet = new Wallet(); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java index abe69e76ff2..ed2121d360f 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/TransactionsMsgHandlerTest.java @@ -337,6 +337,91 @@ public void testDuplicateTransactionRejected() throws Exception { } } + @Test + public void testInvalidSigLength() throws Exception { + TransactionsMsgHandler handler = new TransactionsMsgHandler(); + handler.init(); + try { + PeerConnection peer = Mockito.mock(PeerConnection.class); + + BalanceContract.TransferContract transferContract = BalanceContract.TransferContract + .newBuilder() + .setAmount(10) + .setOwnerAddress(ByteString.copyFrom(ByteArray.fromHexString("121212a9cf"))) + .setToAddress(ByteString.copyFrom(ByteArray.fromHexString("232323a9cf"))) + .build(); + + // signature shorter than 65 bytes → BAD_TRX + Protocol.Transaction shortSigTrx = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .addContract(Protocol.Transaction.Contract.newBuilder() + .setType(Protocol.Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(transferContract)).build()) + .build()) + .addSignature(ByteString.copyFrom(new byte[64])) + .build(); + + List shortList = new ArrayList<>(); + shortList.add(shortSigTrx); + stubAdvInvRequest(peer, new TransactionsMessage(shortList)); + P2pException shortEx = Assert.assertThrows(P2pException.class, + () -> handler.processMessage(peer, new TransactionsMessage(shortList))); + Assert.assertEquals(TypeEnum.BAD_TRX, shortEx.getType()); + + // signature longer than 68 bytes → BAD_TRX + Protocol.Transaction longSigTrx = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .setRefBlockNum(1) + .addContract(Protocol.Transaction.Contract.newBuilder() + .setType(Protocol.Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(transferContract)).build()) + .build()) + .addSignature(ByteString.copyFrom(new byte[69])) + .build(); + + List longList = new ArrayList<>(); + longList.add(longSigTrx); + stubAdvInvRequest(peer, new TransactionsMessage(longList)); + P2pException longEx = Assert.assertThrows(P2pException.class, + () -> handler.processMessage(peer, new TransactionsMessage(longList))); + Assert.assertEquals(TypeEnum.BAD_TRX, longEx.getType()); + + // exactly 65 bytes → passes the length check (no P2pException from check) + Protocol.Transaction validSigTrx = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .setRefBlockNum(2) + .addContract(Protocol.Transaction.Contract.newBuilder() + .setType(Protocol.Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(transferContract)).build()) + .build()) + .addSignature(ByteString.copyFrom(new byte[65])) + .build(); + + List validList = new ArrayList<>(); + validList.add(validSigTrx); + stubAdvInvRequest(peer, new TransactionsMessage(validList)); + handler.processMessage(peer, new TransactionsMessage(validList)); + + // 68 bytes (upper bound) also passes the length check + Protocol.Transaction paddedSigTrx = Protocol.Transaction.newBuilder() + .setRawData(Protocol.Transaction.raw.newBuilder() + .setRefBlockNum(3) + .addContract(Protocol.Transaction.Contract.newBuilder() + .setType(Protocol.Transaction.Contract.ContractType.TransferContract) + .setParameter(Any.pack(transferContract)).build()) + .build()) + .addSignature(ByteString.copyFrom(new byte[68])) + .build(); + + List paddedList = new ArrayList<>(); + paddedList.add(paddedSigTrx); + stubAdvInvRequest(peer, new TransactionsMessage(paddedList)); + handler.processMessage(peer, new TransactionsMessage(paddedList)); + } finally { + handler.close(); + } + } + @Test public void testIsBusyWithCachedTransactions() throws Exception { TransactionsMsgHandler handler = new TransactionsMsgHandler(); diff --git a/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java b/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java index 8585244b941..7c28757bd5c 100644 --- a/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java +++ b/framework/src/test/java/org/tron/core/net/services/RelayServiceTest.java @@ -220,6 +220,30 @@ private void testCheckHelloMessage() { boolean res = service.checkHelloMessage(helloMessage, c1); Assert.assertTrue(res); + + HelloMessage shortSigMsg = new HelloMessage(node, System.currentTimeMillis(), + ChainBaseManager.getChainBaseManager()); + shortSigMsg.setHelloMessage(shortSigMsg.getHelloMessage().toBuilder() + .setAddress(address) + .setSignature(ByteString.copyFrom(new byte[64])) + .build()); + Assert.assertFalse(service.checkHelloMessage(shortSigMsg, c1)); + + HelloMessage longSigMsg = new HelloMessage(node, System.currentTimeMillis(), + ChainBaseManager.getChainBaseManager()); + longSigMsg.setHelloMessage(longSigMsg.getHelloMessage().toBuilder() + .setAddress(address) + .setSignature(ByteString.copyFrom(new byte[69])) + .build()); + Assert.assertFalse(service.checkHelloMessage(longSigMsg, c1)); + + HelloMessage emptySigMsg = new HelloMessage(node, System.currentTimeMillis(), + ChainBaseManager.getChainBaseManager()); + emptySigMsg.setHelloMessage(emptySigMsg.getHelloMessage().toBuilder() + .setAddress(address) + .setSignature(ByteString.EMPTY) + .build()); + Assert.assertFalse(service.checkHelloMessage(emptySigMsg, c1)); } catch (Exception e) { logger.info("", e); assert false;