fix(chainbase): strengthen signature length validation

actuator/src/main/java/org/tron/core/utils/TransactionUtil.java

@@ -225,7 +225,8 @@ public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
           List<ByteString> approveList = new ArrayList<>();
           long currentWeight = TransactionCapsule.checkWeight(permission, trx.getSignatureList(),
               Sha256Hash.hash(CommonParameter.getInstance()
-                  .isECKeyCryptoEngine(), trx.getRawData().toByteArray()), approveList);
+                  .isECKeyCryptoEngine(), trx.getRawData().toByteArray()), approveList,
+              chainBaseManager.getDynamicPropertiesStore());
           tswBuilder.addAllApprovedList(approveList);
           tswBuilder.setCurrentWeight(currentWeight);
         }

chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java

@@ -185,9 +185,14 @@ private Sha256Hash getRawHash() {
   public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
       AccountStore accountStore) throws ValidateSignatureException {
     try {
+      ByteString witnessSig = block.getBlockHeader().getWitnessSignature();
+      if (!SignUtils.isValidLength(witnessSig.size(),
+          dynamicPropertiesStore.isAllowTvmOsaka())) {
+        throw new ValidateSignatureException(
+            "Witness signature size is " + witnessSig.size());
+      }
       byte[] sigAddress = SignUtils.signatureToAddress(getRawHash().getBytes(),
-          TransactionCapsule.getBase64FromByteString(
-              block.getBlockHeader().getWitnessSignature()),
+          TransactionCapsule.getBase64FromByteString(witnessSig),
           CommonParameter.getInstance().isECKeyCryptoEngine());
       byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress()
           .toByteArray();

chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java

@@ -33,6 +33,7 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
@@ -54,6 +55,7 @@
 import org.tron.common.utils.Sha256Hash;
 import org.tron.core.actuator.TransactionFactory;
 import org.tron.core.config.Parameter;
+import org.tron.core.config.Parameter.ChainConstant;
 import org.tron.core.db.TransactionContext;
 import org.tron.core.db.TransactionTrace;
 import org.tron.core.exception.BadItemException;
@@ -230,8 +232,9 @@ public static long getWeight(Permission permission, byte[] address) {
    *  @see ForkController#init(org.tron.core.ChainBaseManager)
    */
   public static long checkWeight(Permission permission, List<ByteString> sigs, byte[] hash,
-      List<ByteString> approveList)
+      List<ByteString> approveList, DynamicPropertiesStore dynamicPropertiesStore)
       throws SignatureException, PermissionException, SignatureFormatException {
+    Objects.requireNonNull(dynamicPropertiesStore, "dynamicPropertiesStore");
     long currentWeight = 0;
     if (sigs.size() > permission.getKeysCount()) {
       throw new PermissionException(
@@ -240,9 +243,9 @@ public static long checkWeight(Permission permission, List<ByteString> sigs, byt
     }
     HashMap addMap = new HashMap();
     for (ByteString sig : sigs) {
-      if (sig.size() < 65) {
-        throw new SignatureFormatException(
-            "Signature size is " + sig.size());
+      if (!SignUtils.isValidLength(sig.size(),
+          dynamicPropertiesStore.isAllowTvmOsaka())) {
+        throw new SignatureFormatException("Signature size is " + sig.size());
       }
       String base64 = TransactionCapsule.getBase64FromByteString(sig);
       byte[] address = SignUtils
@@ -487,7 +490,8 @@ public static boolean validateSignature(Transaction transaction,
       throw new PermissionException("permission isn't exit");
     }
     checkPermission(permissionId, permission, contract);
-    long weight = checkWeight(permission, transaction.getSignatureList(), hash, null);
+    long weight = checkWeight(permission, transaction.getSignatureList(), hash, null,
+        dynamicPropertiesStore);
     if (weight >= permission.getThreshold()) {
       return true;
     }
@@ -583,7 +587,8 @@ public void sign(byte[] privateKey) {
     this.transaction = this.transaction.toBuilder().addSignature(sig).build();
   }
 
-  public void addSign(byte[] privateKey, AccountStore accountStore)
+  public void addSign(byte[] privateKey, AccountStore accountStore,
+      DynamicPropertiesStore dynamicPropertiesStore)
       throws PermissionException, SignatureException, SignatureFormatException {
     Transaction.Contract contract = this.transaction.getRawData().getContract(0);
     int permissionId = contract.getPermissionId();
@@ -604,7 +609,7 @@ public void addSign(byte[] privateKey, AccountStore accountStore)
     if (this.transaction.getSignatureCount() > 0) {
       checkWeight(permission, this.transaction.getSignatureList(),
           this.getTransactionId().getBytes(),
-          approveList);
+          approveList, dynamicPropertiesStore);
       if (approveList.contains(ByteString.copyFrom(address))) {
         throw new PermissionException(encode58Check(address) + " had signed!");
       }
@@ -620,8 +625,9 @@ public void addSign(byte[] privateKey, AccountStore accountStore)
         .signHash(getTransactionId().getBytes())));
     this.transaction = this.transaction.toBuilder().addSignature(sig).build();
   }
-
-  private static void checkPermission(int permissionId, Permission permission, Transaction.Contract contract) throws PermissionException {
+
+  private static void checkPermission(int permissionId, Permission permission,
+      Transaction.Contract contract) throws PermissionException {
     if (permissionId != 0) {
       if (permission.getType() != PermissionType.Active) {
         throw new PermissionException("Permission type is error");
@@ -703,7 +709,7 @@ public boolean validateSignature(AccountStore accountStore,
         }
       }
       isVerified = true;
-    }  
+    }
     return true;
   }
 

chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java

@@ -3011,6 +3011,10 @@ public void saveAllowTvmOsaka(long value) {
     this.put(ALLOW_TVM_OSAKA, new BytesCapsule(ByteArray.fromLong(value)));
   }
 
+  public boolean isAllowTvmOsaka() {
+    return getAllowTvmOsaka() == 1L;
+  }
+
   public long getAllowTvmPrague() {
     return Optional.ofNullable(getUnchecked(ALLOW_TVM_PRAGUE))
         .map(BytesCapsule::getData)

common/src/main/java/org/tron/core/config/Parameter.java

@@ -69,6 +69,11 @@ public class ChainConstant {
     public static final int MAX_VOTE_NUMBER = 30;
     public static final int SOLIDIFIED_THRESHOLD = 70; // 70%
     public static final int PRIVATE_KEY_LENGTH = 64;
+    public static final int MIN_SIGNATURE_SIZE = 65;
+    // Canonical ECDSA signature is 65 bytes (r||s||v). 68 = 65 + up to 3 trailing
+    // padding bytes; this window accommodates historical non-canonical encodings
+    // observed on chain. Long-term goal is to tighten this back to a strict 65.
+    public static final int MAX_SIGNATURE_SIZE = 68;
     public static final int BLOCK_SIZE = 2_000_000;
     public static final long CLOCK_MAX_DELAY = 3600000; // 3600 * 1000 ms
     public static final int BLOCK_PRODUCE_TIMEOUT_PERCENT = 50; // 50%

consensus/src/main/java/org/tron/consensus/pbft/PbftMessageHandle.java

@@ -129,7 +129,7 @@ public void onPrePrepare(PbftMessage message) {
       PbftMessage paMessage = message.buildPrePareMessage(miner);
       forwardMessage(paMessage);
       try {
-        paMessage.analyzeSignature();
+        paMessage.analyzeSignature(chainBaseManager.getDynamicPropertiesStore());
       } catch (SignatureException e) {
         logger.error("", e);
       }
@@ -175,7 +175,7 @@ public synchronized void onPrepare(PbftMessage message) {
           doneMsg.put(message.getNo(), cmMessage);
           forwardMessage(cmMessage);
           try {
-            cmMessage.analyzeSignature();
+            cmMessage.analyzeSignature(chainBaseManager.getDynamicPropertiesStore());
           } catch (SignatureException e) {
             logger.error("", e);
           }
@@ -316,4 +316,4 @@ public void run() {
       }
     }, 10, 1000);
   }
-}
+}

consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java

@@ -1,17 +1,20 @@
 package org.tron.consensus.pbft.message;
 
+import com.google.protobuf.ByteString;
 import com.google.protobuf.InvalidProtocolBufferException;
 import java.io.IOException;
 import java.security.SignatureException;
 import java.util.stream.Collectors;
 import org.bouncycastle.util.encoders.Hex;
 import org.tron.common.crypto.ECKey;
+import org.tron.common.crypto.SignUtils;
 import org.tron.common.overlay.message.Message;
 import org.tron.common.utils.ByteUtil;
 import org.tron.common.utils.Sha256Hash;
 import org.tron.common.utils.StringUtil;
 import org.tron.core.capsule.TransactionCapsule;
 import org.tron.core.exception.P2pException;
+import org.tron.core.store.DynamicPropertiesStore;
 import org.tron.protos.Protocol.PBFTMessage;
 import org.tron.protos.Protocol.PBFTMessage.DataType;
 import org.tron.protos.Protocol.SRL;
@@ -94,10 +97,16 @@ public DataType getDataType() {
 
   public abstract String getNo();
 
-  public void analyzeSignature() throws SignatureException {
+  public void analyzeSignature(DynamicPropertiesStore dynamicPropertiesStore)
+      throws SignatureException {
+    ByteString signature = getPbftMessage().getSignature();
+    if (!SignUtils.isValidLength(signature.size(),
+        dynamicPropertiesStore.isAllowTvmOsaka())) {
+      throw new SignatureException("PBFT signature size is " + signature.size());
+    }
     byte[] hash = Sha256Hash.hash(true, getPbftMessage().getRawData().toByteArray());
     publicKey = ECKey.signatureToAddress(hash, TransactionCapsule
-        .getBase64FromByteString(getPbftMessage().getSignature()));
+        .getBase64FromByteString(signature));
   }
 
   @Override

crypto/src/main/java/org/tron/common/crypto/SignUtils.java

@@ -5,9 +5,15 @@
 import org.tron.common.crypto.ECKey.ECDSASignature;
 import org.tron.common.crypto.sm2.SM2;
 import org.tron.common.crypto.sm2.SM2.SM2Signature;
+import org.tron.core.config.Parameter.ChainConstant;
 
 public class SignUtils {
 
+  public static boolean isValidLength(int size, boolean osakaAllowed) {
+    return size >= ChainConstant.MIN_SIGNATURE_SIZE
+        && (!osakaAllowed || size <= ChainConstant.MAX_SIGNATURE_SIZE);
+  }
+
   public static SignInterface getGeneratedRandomSign(
       SecureRandom secureRandom, boolean isECKeyCryptoEngine) {
     if (isECKeyCryptoEngine) {

framework/src/main/java/org/tron/core/Wallet.java

@@ -160,6 +160,7 @@
 import org.tron.core.capsule.VotesCapsule;
 import org.tron.core.capsule.WitnessCapsule;
 import org.tron.core.capsule.utils.MarketUtils;
+import org.tron.core.config.Parameter.ChainConstant;
 import org.tron.core.config.args.Args;
 import org.tron.core.db.BandwidthProcessor;
 import org.tron.core.db.BlockIndexStore;
@@ -644,7 +645,8 @@ public TransactionApprovedList getTransactionApprovedList(Transaction trx) {
           byte[] hash = Sha256Hash.hash(CommonParameter
               .getInstance().isECKeyCryptoEngine(), trx.getRawData().toByteArray());
           for (ByteString sig : trx.getSignatureList()) {
-            if (sig.size() < 65) {
+            if (!SignUtils.isValidLength(sig.size(),
+                chainBaseManager.getDynamicPropertiesStore().isAllowTvmOsaka())) {
               throw new SignatureFormatException(
                   "Signature size is " + sig.size());
             }

framework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.java

@@ -19,6 +19,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.tron.common.crypto.ECKey;
+import org.tron.common.crypto.SignUtils;
 import org.tron.common.es.ExecutorServiceManager;
 import org.tron.common.utils.ByteArray;
 import org.tron.common.utils.Sha256Hash;
@@ -122,20 +123,17 @@ private void processPBFTCommitMessage(PbftCommitMessage pbftCommitMessage) {
   private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
       List<ByteString> currentSrList) {
     //valid sr list
+    if (srSignList.size() < Param.getInstance().getAgreeNodeCount()) {
+      return false;
+    }
     if (srSignList.size() != 0) {
-      Set<ByteString> srSignSet = new ConcurrentSet();
-      srSignSet.addAll(srSignList);
-      if (srSignSet.size() < Param.getInstance().getAgreeNodeCount()) {
-        logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", srSignSet.size(),
-            Param.getInstance().getAgreeNodeCount());
-        return false;
-      }
       byte[] dataHash = Sha256Hash.hash(true, raw.toByteArray());
       Set<ByteString> srSet = Sets.newHashSet(currentSrList);
+      Set<ByteString> validSrAddressSet = new ConcurrentSet();
       List<Future<Boolean>> futureList = new ArrayList<>();
       for (ByteString sign : srSignList) {
         futureList.add(executorService.submit(
-            new ValidPbftSignTask(raw.getViewN(), srSignSet, dataHash, srSet, sign)));
+            new ValidPbftSignTask(raw.getViewN(), validSrAddressSet, dataHash, srSet, sign)));
       }
       for (Future<Boolean> future : futureList) {
         try {
@@ -146,7 +144,9 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
           logger.error("", e);
         }
       }
-      if (srSignSet.size() != 0) {
+      if (validSrAddressSet.size() < Param.getInstance().getAgreeNodeCount()) {
+        logger.error("sr sign count {} < sr count * 2/3 + 1 == {}", validSrAddressSet.size(),
+            Param.getInstance().getAgreeNodeCount());
         return false;
       }
     }
@@ -156,15 +156,15 @@ private boolean validPbftSign(Raw raw, List<ByteString> srSignList,
   private class ValidPbftSignTask implements Callable<Boolean> {
 
     private long viewN;
-    private Set<ByteString> srSignSet;
+    private Set<ByteString> validSrAddressSet;
     private byte[] dataHash;
     private Set<ByteString> srSet;
     private ByteString sign;
 
-    ValidPbftSignTask(long viewN, Set<ByteString> srSignSet,
+    ValidPbftSignTask(long viewN, Set<ByteString> validSrAddressSet,
         byte[] dataHash, Set<ByteString> srSet, ByteString sign) {
       this.viewN = viewN;
-      this.srSignSet = srSignSet;
+      this.validSrAddressSet = validSrAddressSet;
       this.dataHash = dataHash;
       this.srSet = srSet;
       this.sign = sign;
@@ -173,14 +173,18 @@ private class ValidPbftSignTask implements Callable<Boolean> {
     @Override
     public Boolean call() throws Exception {
       try {
+        if (!SignUtils.isValidLength(sign.size(),
+            chainBaseManager.getDynamicPropertiesStore().isAllowTvmOsaka())) {
+          throw new SignatureException("PBFT signature size is " + sign.size());
+        }
         byte[] srAddress = ECKey.signatureToAddress(dataHash,
             TransactionCapsule.getBase64FromByteString(sign));
         if (!srSet.contains(ByteString.copyFrom(srAddress))) {
           logger.error("valid sr signature fail,error sr address:{}",
               ByteArray.toHexString(srAddress));
           return false;
         }
-        srSignSet.remove(sign);
+        validSrAddressSet.add(ByteString.copyFrom(srAddress));
       } catch (SignatureException e) {
         logger.error("viewN {} valid sr list sign fail!", viewN, e);
         return false;

framework/src/main/java/org/tron/core/net/messagehandler/PbftMsgHandler.java

@@ -11,6 +11,7 @@
 import org.tron.consensus.pbft.PbftManager;
 import org.tron.consensus.pbft.message.PbftBaseMessage;
 import org.tron.consensus.pbft.message.PbftMessage;
+import org.tron.core.ChainBaseManager;
 import org.tron.core.config.args.Args;
 import org.tron.core.exception.P2pException;
 import org.tron.core.net.TronNetDelegate;
@@ -32,6 +33,9 @@ public class PbftMsgHandler {
   @Autowired
   private TronNetDelegate tronNetDelegate;
 
+  @Autowired
+  private ChainBaseManager chainBaseManager;
+
   public void processMessage(PeerConnection peer, PbftMessage msg) throws Exception {
     if (!tronNetDelegate.allowPBFT()) {
       return;
@@ -50,7 +54,7 @@ public void processMessage(PeerConnection peer, PbftMessage msg) throws Exceptio
         && currentEpoch - msg.getEpoch() > expireEpoch) {
       return;
     }
-    msg.analyzeSignature();
+    msg.analyzeSignature(chainBaseManager.getDynamicPropertiesStore());
     String key = buildKey(msg);
     Lock lock = striped.get(key);
     try {
@@ -79,4 +83,4 @@ private String buildKey(PbftBaseMessage msg) {
     return msg.getKey() + msg.getPbftMessage().getRawData().getMsgType().toString();
   }
 
-}
+}

framework/src/main/java/org/tron/core/net/service/relay/RelayService.java

@@ -152,10 +152,15 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) {
 
     boolean flag;
     try {
+      ByteString signature = msg.getSignature();
+      if (!SignUtils.isValidLength(signature.size(),
+          chainBaseManager.getDynamicPropertiesStore().isAllowTvmOsaka())) {
+        return false;
+      }
       Sha256Hash hash = Sha256Hash.of(CommonParameter
           .getInstance().isECKeyCryptoEngine(), ByteArray.fromLong(msg.getTimestamp()));
       String sig =
-          TransactionCapsule.getBase64FromByteString(msg.getSignature());
+          TransactionCapsule.getBase64FromByteString(signature);
       byte[] sigAddress = SignUtils.signatureToAddress(hash.getBytes(), sig,
           Args.getInstance().isECKeyCryptoEngine());
       if (manager.getDynamicPropertiesStore().getAllowMultiSign() != 1) {

framework/src/test/java/org/tron/common/utils/client/utils/TransactionUtils.java

@@ -32,6 +32,7 @@
 import org.tron.core.exception.SignatureFormatException;
 import org.tron.core.services.http.JsonFormat;
 import org.tron.core.store.AccountStore;
+import org.tron.core.store.DynamicPropertiesStore;
 import org.tron.protos.Protocol.Transaction;
 import org.tron.protos.Protocol.Transaction.Contract;
 import org.tron.protos.contract.AccountContract.AccountCreateContract;
@@ -193,11 +194,12 @@ public static String getTransactionSign(String transaction, String priKey,
   }
 
   public static TransactionCapsule addTransactionSign(Transaction transaction, String priKey,
-                                               AccountStore accountStore)
+                                               AccountStore accountStore,
+                                               DynamicPropertiesStore dynamicPropertiesStore)
           throws PermissionException, SignatureException, SignatureFormatException {
     byte[] privateKey = ByteArray.fromHexString(priKey);
     TransactionCapsule trx = new TransactionCapsule(transaction);
-    trx.addSign(privateKey, accountStore);
+    trx.addSign(privateKey, accountStore, dynamicPropertiesStore);
     return trx;
   }