feat(crypto): add strict ECDSA validation

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

@@ -968,6 +968,21 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore,
         }
         break;
       }
+      case ALLOW_STRICT_ECDSA_VALIDATION: {
+        if (!forkController.pass(ForkBlockVersionEnum.VERSION_4_8_3)) {
+          throw new ContractValidateException(
+              "Bad chain parameter id [ALLOW_STRICT_ECDSA_VALIDATION]");
+        }
+        if (dynamicPropertiesStore.allowStrictEcdsaValidation()) {
+          throw new ContractValidateException(
+              "[ALLOW_STRICT_ECDSA_VALIDATION] has been valid, no need to propose again");
+        }
+        if (value != 1) {
+          throw new ContractValidateException(
+              "This value[ALLOW_STRICT_ECDSA_VALIDATION] is only allowed to be 1");
+        }
+        break;
+      }
       case CLOSE_EXCHANGE: {
         if (!forkController.pass(ForkBlockVersionEnum.VERSION_4_8_3)) {
           throw new ContractValidateException("Bad chain parameter id [CLOSE_EXCHANGE]");
@@ -1078,6 +1093,7 @@ public enum ProposalType {         // current value, value range
     ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1
     ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1
     ALLOW_OPTIMIZE_TVM_STORAGE(99), // 0, 1
+    ALLOW_STRICT_ECDSA_VALIDATION(100), // 0, 1
     CLOSE_EXCHANGE(101); // 0, 1, 2
     private long code;
 

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

@@ -18,35 +18,19 @@
 import static org.tron.common.crypto.Hash.sha3omit12;
 import static org.tron.common.math.Maths.max;
 import static org.tron.core.config.Parameter.ChainConstant.DELEGATE_COST_BASE_SIZE;
-import static org.tron.core.Constant.PER_SIGN_LENGTH;
 import static org.tron.core.config.Parameter.ChainConstant.TRX_PRECISION;
 
 import com.google.common.base.CaseFormat;
 import com.google.common.primitives.Longs;
 import com.google.protobuf.Any;
 import com.google.protobuf.ByteString;
-import java.security.SignatureException;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Objects;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.ArrayUtils;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
-import org.tron.api.GrpcAPI.Return;
-import org.tron.api.GrpcAPI.Return.response_code;
-import org.tron.api.GrpcAPI.TransactionExtention;
-import org.tron.api.GrpcAPI.TransactionSignWeight;
-import org.tron.api.GrpcAPI.TransactionSignWeight.Result;
 import org.tron.common.utils.Sha256Hash;
-import org.tron.core.ChainBaseManager;
-import org.tron.core.capsule.AccountCapsule;
-import org.tron.core.capsule.TransactionCapsule;
 import org.tron.core.exception.PermissionException;
-import org.tron.core.exception.SignatureFormatException;
 import org.tron.core.store.DynamicPropertiesStore;
 import org.tron.protos.Protocol.Permission;
-import org.tron.protos.Protocol.Permission.PermissionType;
 import org.tron.protos.Protocol.Transaction;
 import org.tron.protos.Protocol.Transaction.Contract;
 import org.tron.protos.Protocol.Transaction.Result.contractResult;
@@ -66,9 +50,6 @@ public class TransactionUtil {
   private static final int MAX_ASSET_DESCRIPTION_LEN = 200;
   private static final int MAX_URL_LEN = 256;
 
-  @Autowired
-  private ChainBaseManager chainBaseManager;
-
   public static boolean validAccountName(byte[] accountName) {
     return validBytes(accountName, MAX_ACCOUNT_NAME_LEN, true);
   }
@@ -182,95 +163,6 @@ public static String makeUpperCamelMethod(String originName) {
         .replace("_", "");
   }
 
-  public static Transaction truncateSignatures(Transaction trx) {
-    Transaction.Builder builder = trx.toBuilder().clearSignature();
-    for (ByteString sig : trx.getSignatureList()) {
-      if (sig.size() > PER_SIGN_LENGTH) {
-        builder.addSignature(ByteString.copyFrom(sig.substring(0, PER_SIGN_LENGTH).toByteArray()));
-      } else {
-        builder.addSignature(sig);
-      }
-    }
-    return builder.build();
-  }
-
-  public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
-    TransactionSignWeight.Builder tswBuilder = TransactionSignWeight.newBuilder();
-    Result.Builder resultBuilder = Result.newBuilder();
-    if (trx.getSignatureCount() > chainBaseManager.getDynamicPropertiesStore()
-        .getTotalSignNum()) {
-      resultBuilder.setCode(Result.response_code.OTHER_ERROR);
-      resultBuilder.setMessage("too many signatures");
-      tswBuilder.setResult(resultBuilder);
-      return tswBuilder.build();
-    }
-
-    trx = truncateSignatures(trx);
-    TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder();
-    trxExBuilder.setTransaction(trx);
-    trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(trx.getRawData().toByteArray())));
-    Return.Builder retBuilder = Return.newBuilder();
-    retBuilder.setResult(true).setCode(response_code.SUCCESS);
-    trxExBuilder.setResult(retBuilder);
-    tswBuilder.setTransaction(trxExBuilder);
-
-    if (trx.getRawData().getContractCount() == 0) {
-      resultBuilder.setCode(Result.response_code.OTHER_ERROR);
-      resultBuilder.setMessage("Invalid transaction: no valid contract");
-    } else {
-      try {
-        Contract contract = trx.getRawData().getContract(0);
-        byte[] owner = TransactionCapsule.getOwner(contract);
-        AccountCapsule account = chainBaseManager.getAccountStore().get(owner);
-        if (Objects.isNull(account)) {
-          throw new PermissionException("Account does not exist!");
-        }
-        int permissionId = contract.getPermissionId();
-        Permission permission = account.getPermissionById(permissionId);
-        if (permission == null) {
-          throw new PermissionException("Permission for this, does not exist!");
-        }
-        if (permissionId != 0) {
-          if (permission.getType() != PermissionType.Active) {
-            throw new PermissionException("Permission type is wrong!");
-          }
-          //check operations
-          if (!checkPermissionOperations(permission, contract)) {
-            throw new PermissionException("Permission denied!");
-          }
-        }
-        tswBuilder.setPermission(permission);
-        if (trx.getSignatureCount() > 0) {
-          List<ByteString> approveList = new ArrayList<>();
-          long currentWeight = TransactionCapsule.checkWeight(permission, trx.getSignatureList(),
-              Sha256Hash.hash(trx.getRawData().toByteArray()), approveList);
-          tswBuilder.addAllApprovedList(approveList);
-          tswBuilder.setCurrentWeight(currentWeight);
-        }
-        if (tswBuilder.getCurrentWeight() >= permission.getThreshold()) {
-          resultBuilder.setCode(Result.response_code.ENOUGH_PERMISSION);
-        } else {
-          resultBuilder.setCode(Result.response_code.NOT_ENOUGH_PERMISSION);
-        }
-      } catch (SignatureFormatException signEx) {
-        resultBuilder.setCode(Result.response_code.SIGNATURE_FORMAT_ERROR);
-        resultBuilder.setMessage(signEx.getMessage());
-      } catch (SignatureException signEx) {
-        resultBuilder.setCode(Result.response_code.COMPUTE_ADDRESS_ERROR);
-        resultBuilder.setMessage(signEx.getMessage());
-      } catch (PermissionException permEx) {
-        resultBuilder.setCode(Result.response_code.PERMISSION_ERROR);
-        resultBuilder.setMessage(permEx.getMessage());
-      } catch (Exception ex) {
-        resultBuilder.setCode(Result.response_code.OTHER_ERROR);
-        resultBuilder.setMessage(ex.getClass() + " : " + ex.getMessage());
-      }
-    }
-
-    tswBuilder.setResult(resultBuilder);
-    return tswBuilder.build();
-  }
-
   public static long estimateConsumeBandWidthSize(DynamicPropertiesStore dps, long balance) {
     DelegateResourceContract.Builder builder;
     if (dps.supportMaxDelegateLockPeriod()) {

actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java

@@ -377,7 +377,8 @@ private static byte[] recoverAddrBySign(byte[] sign, byte[] hash) {
       Rsv rsv = Rsv.fromSignature(sign);
       ECDSASignature signature = SignUtils.fromComponents(rsv.getR(), rsv.getS(), rsv.getV());
       if (signature.validateComponents()) {
-        out = SignUtils.signatureToAddress(hash, signature);
+        out = SignUtils.signatureToAddress(hash, signature,
+            VMConfig.allowStrictEcdsaValidation());
       }
     } catch (Throwable any) {
       logger.info("ECRecover error", any.getMessage());
@@ -609,7 +610,8 @@ public Pair<Boolean, byte[]> execute(byte[] data) {
 
         ECDSASignature signature = SignUtils.fromComponents(r, s, v[31]);
         if (validateV(v) && signature.validateComponents()) {
-          out = new DataWord(SignUtils.signatureToAddress(h, signature));
+          out = new DataWord(SignUtils.signatureToAddress(h, signature,
+              VMConfig.allowStrictEcdsaValidation()));
         }
       } catch (Throwable any) {
       }
@@ -1093,6 +1095,9 @@ public Pair<Boolean, byte[]> execute(byte[] rawData) {
             List<byte[]> executedSignList = new ArrayList<>();
             for (byte[] sign : signatures) {
               byte[] recoveredAddr = recoverAddrBySign(sign, hash);
+              if (recoveredAddr == null) {
+                return Pair.of(true, DATA_FALSE);
+              }
 
               sign = merge(recoveredAddr, sign);
               if (ByteArray.matrixContains(executedSignList, recoveredAddr)) {

actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java

@@ -51,6 +51,7 @@ public static void load(StoreFactory storeFactory, boolean isolate) {
         snapshot.allowTvmOsaka = ds.getAllowTvmOsaka() == 1;
         snapshot.allowHardenResourceCalculation = ds.getAllowHardenResourceCalculation() == 1;
         snapshot.allowOptimizeTvmStorage = ds.getAllowOptimizeTvmStorage() == 1;
+        snapshot.allowStrictEcdsaValidation = ds.allowStrictEcdsaValidation();
         if (isolate) {
           VMConfig.setLocalSnapshot(snapshot);
         } else {

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

@@ -15,6 +15,7 @@
 
 package org.tron.core.capsule;
 
+import static org.tron.core.Constant.PER_SIGN_LENGTH;
 import static org.tron.core.exception.BadBlockException.TypeEnum.CALC_MERKLE_ROOT_FAILED;
 
 import com.google.common.primitives.Longs;
@@ -183,9 +184,14 @@ private Sha256Hash getRawHash() {
   public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
       AccountStore accountStore) throws ValidateSignatureException {
     try {
+      ByteString witnessSignature = block.getBlockHeader().getWitnessSignature();
+      boolean strictEcdsaValidation = dynamicPropertiesStore.allowStrictEcdsaValidation();
+      if (strictEcdsaValidation
+          && !SignUtils.isValidLength(witnessSignature.size())) {
+        throw new ValidateSignatureException("Invalid ECDSA signature format");
+      }
       byte[] sigAddress = SignUtils.signatureToAddress(getRawHash().getBytes(),
-          TransactionCapsule.getBase64FromByteString(
-              block.getBlockHeader().getWitnessSignature()));
+          TransactionCapsule.getBase64FromByteString(witnessSignature), strictEcdsaValidation);
       byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress()
           .toByteArray();
 
@@ -328,20 +334,33 @@ public boolean hasWitnessSignature() {
   public boolean sanitize() {
     boolean blockHasUnknown = !this.block.getUnknownFields().asMap().isEmpty();
     boolean headerHasUnknown = !this.block.getBlockHeader().getUnknownFields().asMap().isEmpty();
-    if (!blockHasUnknown && !headerHasUnknown) {
+    ByteString witnessSignature = this.block.getBlockHeader().getWitnessSignature();
+    boolean hasOverlongWitnessSignature = witnessSignature.size() > PER_SIGN_LENGTH;
+    if (!blockHasUnknown && !headerHasUnknown && !hasOverlongWitnessSignature) {
       return false;
     }
     UnknownFieldSet empty = UnknownFieldSet.getDefaultInstance();
     Block.Builder builder = this.block.toBuilder();
+    BlockHeader.Builder headerBuilder = this.block.getBlockHeader().toBuilder();
     if (blockHasUnknown) {
       builder.setUnknownFields(empty);
     }
     if (headerHasUnknown) {
-      builder.setBlockHeader(this.block.getBlockHeader().toBuilder()
-          .setUnknownFields(empty)
-          .build());
+      headerBuilder.setUnknownFields(empty);
+    }
+    if (hasOverlongWitnessSignature) {
+      // Copy the canonical prefix so it does not retain the oversized signature's backing array.
+      headerBuilder.setWitnessSignature(ByteString.copyFrom(
+          witnessSignature.substring(0, PER_SIGN_LENGTH).toByteArray()));
+    }
+    if (headerHasUnknown || hasOverlongWitnessSignature) {
+      builder.setBlockHeader(headerBuilder.build());
     }
     this.block = builder.build();
+    logger.debug("Sanitized block {}, removed unknown fields: block={}, header={}, "
+            + "witness signature length: {} -> {}",
+        getBlockId().getString(), blockHasUnknown, headerHasUnknown,
+        witnessSignature.size(), this.block.getBlockHeader().getWitnessSignature().size());
     return true;
   }
 

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

@@ -100,8 +100,8 @@ public class TransactionCapsule implements ProtoCapsule<Transaction> {
   private static final long SLOW_SIG_VERIFY_MS = 50;
 
   private Transaction transaction;
-  @Setter
-  private boolean isVerified = false;
+  @Getter
+  private volatile boolean isVerified = false;
   @Setter
   @Getter
   private long blockNum = -1;
@@ -233,6 +233,12 @@ public static long getWeight(Permission permission, byte[] address) {
   public static long checkWeight(Permission permission, List<ByteString> sigs, byte[] hash,
       List<ByteString> approveList)
       throws SignatureException, PermissionException, SignatureFormatException {
+    return checkWeight(permission, sigs, hash, approveList, true);
+  }
+
+  public static long checkWeight(Permission permission, List<ByteString> sigs, byte[] hash,
+      List<ByteString> approveList, boolean strictEcdsaValidation)
+      throws SignatureException, PermissionException, SignatureFormatException {
     long currentWeight = 0;
     if (sigs.size() > permission.getKeysCount()) {
       throw new PermissionException(
@@ -241,12 +247,13 @@ public static long checkWeight(Permission permission, List<ByteString> sigs, byt
     }
     HashMap addMap = new HashMap();
     for (ByteString sig : sigs) {
-      if (sig.size() < 65) {
+      if (sig.size() < 65
+          || (strictEcdsaValidation && !SignUtils.isValidLength(sig.size()))) {
         throw new SignatureFormatException(
             "Signature size is " + sig.size());
       }
       String base64 = TransactionCapsule.getBase64FromByteString(sig);
-      byte[] address = SignUtils.signatureToAddress(hash, base64);
+      byte[] address = SignUtils.signatureToAddress(hash, base64, strictEcdsaValidation);
       long weight = getWeight(permission, address);
       if (weight == 0) {
         throw new PermissionException(
@@ -486,7 +493,9 @@ 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);
+    boolean strictEcdsaValidation = dynamicPropertiesStore.allowStrictEcdsaValidation();
+    long weight = checkWeight(permission, transaction.getSignatureList(), hash, null,
+        strictEcdsaValidation);
     if (weight >= permission.getThreshold()) {
       return true;
     }
@@ -689,7 +698,7 @@ void logSlowSigVerify(long startNs) {
   /**
    * validate signature
    */
-  public boolean validateSignature(AccountStore accountStore,
+  public synchronized boolean validateSignature(AccountStore accountStore,
       DynamicPropertiesStore dynamicPropertiesStore) throws ValidateSignatureException {
     if (!isVerified) {
       //Do not support multi contracts in one transaction
@@ -712,6 +721,10 @@ public boolean validateSignature(AccountStore accountStore,
     return true;
   }
 
+  public synchronized void setVerified(boolean verified) {
+    isVerified = verified;
+  }
+
   public Sha256Hash getTransactionId() {
     if (this.id == null) {
       this.id = getRawHash();

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

@@ -255,6 +255,9 @@ public class DynamicPropertiesStore extends TronStoreWithRevoking<BytesCapsule>
   private static final byte[] ALLOW_HARDEN_EXCHANGE_CALCULATION =
       "ALLOW_HARDEN_EXCHANGE_CALCULATION".getBytes();
 
+  private static final byte[] ALLOW_STRICT_ECDSA_VALIDATION =
+      "ALLOW_STRICT_ECDSA_VALIDATION".getBytes();
+
   private static final byte[] CLOSE_EXCHANGE = "CLOSE_EXCHANGE".getBytes();
 
   private static final byte[] TURKISH_KEY_MIGRATION_DONE =
@@ -3087,6 +3090,21 @@ public boolean allowHardenExchangeCalculation() {
     return getAllowHardenExchangeCalculation() == 1L;
   }
 
+  public long getAllowStrictEcdsaValidation() {
+    return Optional.ofNullable(getUnchecked(ALLOW_STRICT_ECDSA_VALIDATION))
+        .map(BytesCapsule::getData)
+        .map(ByteArray::toLong)
+        .orElse(0L);
+  }
+
+  public void saveAllowStrictEcdsaValidation(long value) {
+    this.put(ALLOW_STRICT_ECDSA_VALIDATION, new BytesCapsule(ByteArray.fromLong(value)));
+  }
+
+  public boolean allowStrictEcdsaValidation() {
+    return getAllowStrictEcdsaValidation() == 1L;
+  }
+
   public void saveTurkishKeyMigrationDone(long num) {
     this.put(TURKISH_KEY_MIGRATION_DONE,
         new BytesCapsule(ByteArray.fromLong(num)));

common/src/main/java/org/tron/common/prometheus/MetricKeys.java

@@ -20,6 +20,7 @@ public static class Counter {
     public static final String SR_SET_CHANGE = "tron:sr_set_change";
     public static final String P2P_ERROR = "tron:p2p_error";
     public static final String P2P_DISCONNECT = "tron:p2p_disconnect";
+    public static final String TX_REJECT = "tron:tx_reject";
     public static final String INTERNAL_SERVICE_FAIL = "tron:internal_service_fail";
     // verification counters for the bounded fetch latency estimator rollout
     public static final String BLOCK_FETCH_ARMED = "tron:block_fetch_armed";

common/src/main/java/org/tron/common/prometheus/MetricLabels.java

@@ -31,6 +31,9 @@ public static class Counter {
     public static final String TXS_FAIL_SIG = "sig";
     public static final String TXS_FAIL_TAPOS = "tapos";
     public static final String TXS_FAIL_DUP = "dup";
+    public static final String TX_REJECT_P2P = "p2p";
+    public static final String TX_REJECT_API = "api";
+    public static final String TX_REJECT_INVALID_SIGNATURE_LENGTH = "invalid_signature_length";
     public static final String SR_ADD = "add";
     public static final String SR_REMOVE = "remove";