fix(net): restrict admission signature length (#6782)

Author: Federico2014

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

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}].
+   *
+   * <p>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) {

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)

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());
+        }
+      }
     }
   }
 

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

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();

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<Protocol.Transaction> 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<Protocol.Transaction> 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<Protocol.Transaction> 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<Protocol.Transaction> 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();

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;