Skip to content

feat(crypto): add strict ECDSA validation #7001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Federico2014 wants to merge 8 commits into
base: release_v4.8.3
Choose a base branch
from
+1,936 −179
Open

feat(crypto): add strict ECDSA validation #7001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b125b72
feat(crypto): add strict ECDSA validation
Federico2014 Aug 5, 2026
63c14c8
fix(block): reset stale signature cache
Federico2014 Aug 5, 2026
7a8ade7
fix(crypto): reject point-at-infinity recovery
Federico2014 Aug 13, 2026
a746a56
fix(crypto): harden signature validation
Federico2014 Sep 4, 2026
8f787f0
fix(crypto): tighten signature validation paths
Federico2014 Sep 24, 2026
1e0b6a0
fix(crypto): resolve release branch conflicts
Federico2014 Oct 8, 2026
dfc2f60
fix(crypto): tighten signature recovery handling
Federico2014 Oct 9, 2026
a72b84e
fix(net): drop invalid signatures per transaction
Federico2014 Oct 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion actuator/src/main/java/org/tron/core/utils/ProposalUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,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;
}
default:
break;
}
Expand Down Expand Up @@ -1045,7 +1060,8 @@ public enum ProposalType { // current value, value range
ALLOW_TVM_OSAKA(96), // 0, 1
ALLOW_HARDEN_RESOURCE_CALCULATION(97), // 0, 1
ALLOW_HARDEN_EXCHANGE_CALCULATION(98), // 0, 1
ALLOW_OPTIMIZE_TVM_STORAGE(99); // 0, 1
ALLOW_OPTIMIZE_TVM_STORAGE(99), // 0, 1
ALLOW_STRICT_ECDSA_VALIDATION(100); // 0, 1
private long code;

ProposalType(long code) {
Expand Down
23 changes: 14 additions & 9 deletions actuator/src/main/java/org/tron/core/utils/TransactionUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
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;
Expand All @@ -38,6 +37,7 @@
import org.tron.api.GrpcAPI.TransactionExtention;
import org.tron.api.GrpcAPI.TransactionSignWeight;
import org.tron.api.GrpcAPI.TransactionSignWeight.Result;
import org.tron.common.crypto.SignUtils;
import org.tron.common.parameter.CommonParameter;
import org.tron.common.utils.Sha256Hash;
import org.tron.core.ChainBaseManager;
Expand Down Expand Up @@ -184,16 +184,15 @@ public static String makeUpperCamelMethod(String originName) {
.replace("_", "");
}

public static Transaction truncateSignatures(Transaction trx) {
Transaction.Builder builder = trx.toBuilder().clearSignature();
Comment thread
Federico2014 marked this conversation as resolved.
/**
* Checks every supplied query signature before recovery, without rewriting the transaction.
*/
public static void validateSignatureLengths(Transaction trx) throws SignatureFormatException {
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);
if (!SignUtils.isValidLength(sig.size())) {
throw new SignatureFormatException("Signature size is " + sig.size());
Comment thread
Federico2014 marked this conversation as resolved.
}
}
return builder.build();
}

public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
Expand All @@ -207,7 +206,13 @@ public TransactionSignWeight getTransactionSignWeight(Transaction trx) {
return tswBuilder.build();
}

trx = truncateSignatures(trx);
try {
validateSignatureLengths(trx);
} catch (SignatureFormatException e) {
return tswBuilder.setResult(resultBuilder.setCode(Result.response_code.SIGNATURE_FORMAT_ERROR)
.setMessage(e.getMessage())).build();
}

TransactionExtention.Builder trxExBuilder = TransactionExtention.newBuilder();
trxExBuilder.setTransaction(trx);
trxExBuilder.setTxid(ByteString.copyFrom(Sha256Hash.hash(CommonParameter
Expand Down
11 changes: 8 additions & 3 deletions actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ private static byte[] recoverAddrBySign(byte[] sign, byte[] hash) {
CommonParameter.getInstance().isECKeyCryptoEngine());
if (signature.validateComponents()) {
out = SignUtils.signatureToAddress(hash, signature,
CommonParameter.getInstance().isECKeyCryptoEngine());
CommonParameter.getInstance().isECKeyCryptoEngine(),
VMConfig.allowStrictEcdsaValidation());
}
} catch (Throwable any) {
logger.info("ECRecover error", any.getMessage());
Expand Down Expand Up @@ -616,8 +617,9 @@ public Pair<Boolean, byte[]> execute(byte[] data) {
SignatureInterface signature = SignUtils.fromComponents(r, s, v[31]
, CommonParameter.getInstance().isECKeyCryptoEngine());
if (validateV(v) && signature.validateComponents()) {
out = new DataWord(SignUtils.signatureToAddress(h, signature
, CommonParameter.getInstance().isECKeyCryptoEngine()));
out = new DataWord(SignUtils.signatureToAddress(h, signature,
CommonParameter.getInstance().isECKeyCryptoEngine(),
VMConfig.allowStrictEcdsaValidation()));
}
} catch (Throwable any) {
}
Expand Down Expand Up @@ -1102,6 +1104,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)) {
Expand Down
1 change: 1 addition & 0 deletions actuator/src/main/java/org/tron/core/vm/config/ConfigLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
29 changes: 23 additions & 6 deletions chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -186,10 +187,17 @@ private Sha256Hash getRawHash() {
public boolean validateSignature(DynamicPropertiesStore dynamicPropertiesStore,
AccountStore accountStore) throws ValidateSignatureException {
try {
ByteString witnessSignature = block.getBlockHeader().getWitnessSignature();
boolean strictEcdsaValidation = CommonParameter.getInstance().isECKeyCryptoEngine()
&& dynamicPropertiesStore.allowStrictEcdsaValidation();
Comment on lines +191 to +192

@3for 3for Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD] Revalidate the witness signature after the parent-block state is established

BlockMsgHandler.processBlock() verifies the witness signature before acquiring blockLock, so the proposal activation state may differ from the state when the block is actually applied.

For example, block B may pass verification under the old rules while its parent block A is still being processed. After A activates strict signature validation, B can be applied without revalidation in Manager.processBlock(), potentially causing inconsistent block acceptance between broadcast and synchronization paths.

Suggested fix: Revalidate the witness signature at the beginning of Manager.processBlock(), where the lock is already held and the parent-block state is established:

if (!block.generatedByMyself
    && !block.validateSignature(getDynamicPropertiesStore(), getAccountStore())) {
  throw new ValidateSignatureException(
      "block " + block.getNum() + " signature invalid");
}

Please also add a regression test covering B passing preliminary verification under the old rules but being rejected after A activates strict validation.

@Federico2014 Federico2014 Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3for Revalidation in Manager.processBlock() would catch stale successful checks, but it cannot address valid blocks already rejected by network-layer checks using the wrong permission or branch state. It also adds another witness-signature verification on the serialized block-application path.

sanitize() already mitigates the signature-length difference between legacy and strict rules. The remaining scalar-bound and point-at-infinity cases do not arise from normal SR signing and primarily concern deliberately crafted signatures.

I suggest handling this in a separate PR: move authoritative state-dependent validation into Manager after the correct parent state is established, with corresponding changes to synchronization, broadcasting, caching, and peer attribution. This would address both stale acceptance and premature rejection while avoiding duplicate witness verification.

if (strictEcdsaValidation
&& !SignUtils.isValidLength(witnessSignature.size())) {
Comment thread
Federico2014 marked this conversation as resolved.
throw new ValidateSignatureException("Invalid ECDSA signature format");
}
byte[] sigAddress = SignUtils.signatureToAddress(getRawHash().getBytes(),
TransactionCapsule.getBase64FromByteString(
block.getBlockHeader().getWitnessSignature()),
CommonParameter.getInstance().isECKeyCryptoEngine());
witnessSignature),
CommonParameter.getInstance().isECKeyCryptoEngine(), strictEcdsaValidation);
byte[] witnessAccountAddress = block.getBlockHeader().getRawData().getWitnessAddress()
.toByteArray();

Expand Down Expand Up @@ -332,18 +340,27 @@ 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()));
}
Comment thread
3for marked this conversation as resolved.
if (headerHasUnknown || hasOverlongWitnessSignature) {
builder.setBlockHeader(headerBuilder.build());
}
this.block = builder.build();
return true;
Expand Down
27 changes: 21 additions & 6 deletions chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -241,13 +247,15 @@ 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, CommonParameter.getInstance().isECKeyCryptoEngine());
.signatureToAddress(hash, base64, CommonParameter.getInstance().isECKeyCryptoEngine(),
strictEcdsaValidation);
long weight = getWeight(permission, address);
if (weight == 0) {
throw new PermissionException(
Expand Down Expand Up @@ -488,7 +496,10 @@ 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 = CommonParameter.getInstance().isECKeyCryptoEngine()
&& dynamicPropertiesStore.allowStrictEcdsaValidation();
long weight = checkWeight(permission, transaction.getSignatureList(), hash, null,
strictEcdsaValidation);
if (weight >= permission.getThreshold()) {
return true;
}
Expand Down Expand Up @@ -695,7 +706,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
Expand All @@ -718,6 +729,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();
Expand Down
18 changes: 18 additions & 0 deletions chainbase/src/main/java/org/tron/core/store/DynamicPropertiesStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[] TURKISH_KEY_MIGRATION_DONE =
"TURKISH_KEY_MIGRATION_DONE".getBytes();

Expand Down Expand Up @@ -3073,6 +3076,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)));
Expand Down
1 change: 0 additions & 1 deletion common/src/main/java/org/tron/core/Constant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public class Constant {
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 int PER_SIGN_LENGTH = 65;
public static final int MAX_PER_SIGN_LENGTH = 68;
Comment thread
3for marked this conversation as resolved.
public static final long MAX_CONTRACT_RESULT_SIZE = 2L;

// Smart contract / Energy
Expand Down
5 changes: 3 additions & 2 deletions common/src/main/java/org/tron/core/config/Parameter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ public enum ForkBlockVersionEnum {
VERSION_4_8_1_1(35, 1596780000000L, 70),
VERSION_4_8_2(36, 1596780000000L, 80),
VERSION_4_8_2_2(37, 1596780000000L, 70),
VERSION_4_8_2_3(38, 1596780000000L, 70);
VERSION_4_8_2_3(38, 1596780000000L, 70),
VERSION_4_8_3(39, 1596780000000L, 80);
// if add a version, modify BLOCK_VERSION simultaneously

@Getter
Expand Down Expand Up @@ -81,7 +82,7 @@ public class ChainConstant {
public static final int SINGLE_REPEAT = 1;
public static final int BLOCK_FILLED_SLOTS_NUMBER = 128;
public static final int MAX_FROZEN_NUMBER = 1;
public static final int BLOCK_VERSION = 38;
public static final int BLOCK_VERSION = 39;
public static final long FROZEN_PERIOD = 86_400_000L;
public static final long DELEGATE_PERIOD = 3 * 86_400_000L;
public static final long TRX_PRECISION = 1000_000L;
Expand Down
9 changes: 9 additions & 0 deletions common/src/main/java/org/tron/core/vm/config/VMConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public static class Snapshot {
public boolean allowTvmOsaka;
public boolean allowHardenResourceCalculation;
public boolean allowOptimizeTvmStorage;
public boolean allowStrictEcdsaValidation;
}

// HEAD / block-processing config, written by the consensus path; read by everyone with no
Expand Down Expand Up @@ -203,6 +204,10 @@ public static void initAllowOptimizeTvmStorage(long allow) {
globalSnapshot.allowOptimizeTvmStorage = allow == 1;
}

public static void initAllowStrictEcdsaValidation(long allow) {
globalSnapshot.allowStrictEcdsaValidation = allow == 1;
}

public static boolean getEnergyLimitHardFork() {
return CommonParameter.ENERGY_LIMIT_HARD_FORK;
}
Expand Down Expand Up @@ -314,4 +319,8 @@ public static boolean allowHardenResourceCalculation() {
public static boolean allowOptimizeTvmStorage() {
return current().allowOptimizeTvmStorage;
}

public static boolean allowStrictEcdsaValidation() {
return current().allowStrictEcdsaValidation;
}
}
Loading
Loading