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,15 @@ private Sha256Hash getRawHash() {
   public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
       AccountStore accountStore) throws ValidateSignatureException {
     try {
+      ByteString witnessSig = block.getBlockHeader().getWitnessSignature();
+      if (witnessSig.size() < ChainConstant.MIN_SIGNATURE_SIZE
+          || (dynamicPropertiesStore.getAllowTvmOsaka() == 1
+          && witnessSig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
+        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,10 @@ 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 (sig.size() < ChainConstant.MIN_SIGNATURE_SIZE
+          || (dynamicPropertiesStore.getAllowTvmOsaka() == 1
+          && sig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
+        throw new SignatureFormatException("Signature size is " + sig.size());
       }
       String base64 = TransactionCapsule.getBase64FromByteString(sig);
       byte[] address = SignUtils
@@ -487,7 +491,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 +588,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 +610,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 +626,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 +710,7 @@ public boolean validateSignature(AccountStore accountStore,
         }
       }
       isVerified = true;
-    }  
+    }
     return true;
   }
 

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

@@ -69,6 +69,8 @@ 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;
+    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,5 +1,6 @@
 package org.tron.consensus.pbft.message;
 
+import com.google.protobuf.ByteString;
 import com.google.protobuf.InvalidProtocolBufferException;
 import java.io.IOException;
 import java.security.SignatureException;
@@ -11,7 +12,9 @@
 import org.tron.common.utils.Sha256Hash;
 import org.tron.common.utils.StringUtil;
 import org.tron.core.capsule.TransactionCapsule;
+import org.tron.core.config.Parameter.ChainConstant;
 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,17 @@ public DataType getDataType() {
 
   public abstract String getNo();
 
-  public void analyzeSignature() throws SignatureException {
+  public void analyzeSignature(DynamicPropertiesStore dynamicPropertiesStore)
+      throws SignatureException {
+    ByteString signature = getPbftMessage().getSignature();
+    if (signature.size() < ChainConstant.MIN_SIGNATURE_SIZE
+        || (dynamicPropertiesStore.getAllowTvmOsaka() == 1
+        && signature.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
+      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

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,9 @@ 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 (sig.size() < ChainConstant.MIN_SIGNATURE_SIZE
+                || (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
+                && sig.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
               throw new SignatureFormatException(
                   "Signature size is " + sig.size());
             }

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

@@ -26,6 +26,7 @@
 import org.tron.core.ChainBaseManager;
 import org.tron.core.capsule.BlockCapsule;
 import org.tron.core.capsule.TransactionCapsule;
+import org.tron.core.config.Parameter.ChainConstant;
 import org.tron.core.db.PbftSignDataStore;
 import org.tron.core.exception.P2pException;
 import org.tron.core.net.message.TronMessage;
@@ -173,6 +174,11 @@ private class ValidPbftSignTask implements Callable<Boolean> {
     @Override
     public Boolean call() throws Exception {
       try {
+        if (sign.size() < ChainConstant.MIN_SIGNATURE_SIZE
+            || (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
+            && sign.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
+          throw new SignatureException("PBFT signature size is " + sign.size());
+        }
         byte[] srAddress = ECKey.signatureToAddress(dataHash,
             TransactionCapsule.getBase64FromByteString(sign));
         if (!srSet.contains(ByteString.copyFrom(srAddress))) {

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

@@ -24,6 +24,7 @@
 import org.tron.common.utils.Sha256Hash;
 import org.tron.core.ChainBaseManager;
 import org.tron.core.capsule.TransactionCapsule;
+import org.tron.core.config.Parameter.ChainConstant;
 import org.tron.core.config.args.Args;
 import org.tron.core.db.Manager;
 import org.tron.core.net.TronNetDelegate;
@@ -152,10 +153,16 @@ public boolean checkHelloMessage(HelloMessage message, Channel channel) {
 
     boolean flag;
     try {
+      ByteString signature = msg.getSignature();
+      if (signature.size() < ChainConstant.MIN_SIGNATURE_SIZE
+          || (chainBaseManager.getDynamicPropertiesStore().getAllowTvmOsaka() == 1
+          && signature.size() > ChainConstant.MAX_SIGNATURE_SIZE)) {
+        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;
   }
 

framework/src/test/java/org/tron/core/actuator/ShieldedTransferActuatorTest.java

@@ -207,7 +207,8 @@ public void publicAddressToShieldedAddressSuccess() {
 
       //Add public address sign
       transactionCap = TransactionUtils.addTransactionSign(transactionCap.getInstance(),
-              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore());
+              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore(),
+          dbManager.getDynamicPropertiesStore());
 
       Assert.assertTrue(dbManager.pushTransaction(transactionCap));
     } catch (Exception e) {
@@ -235,7 +236,8 @@ public void publicAddressToPublicAddressAndZereValueOutputSuccess() {
 
       //Add public address sign
       transactionCap = TransactionUtils.addTransactionSign(transactionCap.getInstance(),
-              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore());
+              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore(),
+          dbManager.getDynamicPropertiesStore());
 
       Assert.assertTrue(dbManager.pushTransaction(transactionCap));
     } catch (Exception e) {
@@ -255,7 +257,7 @@ public void publicAddressToShieldedAddressInvalidSign() {
 
       //Add public address sign
       TransactionUtils.addTransactionSign(transactionCap.getInstance(), ADDRESS_TWO_PRIVATE_KEY,
-              dbManager.getAccountStore());
+              dbManager.getAccountStore(), dbManager.getDynamicPropertiesStore());
       Assert.assertTrue(false);
     } catch (PermissionException e) {
       Assert.assertTrue(e instanceof PermissionException);
@@ -395,7 +397,8 @@ public void publicAddressToShieldedAddressNotConsumeBandwidth() {
       TransactionCapsule transactionCap = getPublicToShieldedTransaction();
       //Add public address sign
       transactionCap = TransactionUtils.addTransactionSign(transactionCap.getInstance(),
-              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore());
+              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore(),
+          dbManager.getDynamicPropertiesStore());
 
       AccountCapsule accountCapsule =
           dbManager.getAccountStore().get(ByteArray.fromHexString(PUBLIC_ADDRESS_ONE));
@@ -983,7 +986,8 @@ public void publicToShieldAddressAndShieldToPublicAddressWithZoreValueSuccess()
 
       //Add public address sign
       transactionCapOne = TransactionUtils.addTransactionSign(transactionCapOne.getInstance(),
-              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore());
+              ADDRESS_ONE_PRIVATE_KEY, dbManager.getAccountStore(),
+          dbManager.getDynamicPropertiesStore());
 
       Assert.assertTrue(dbManager.pushTransaction(transactionCapOne));
       AccountCapsule accountCapsuleOne =
@@ -1403,4 +1407,3 @@ public void shieldedTransferValidationWorksWhenApiDisabled() {
     }
   }
 }
-