Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public static List<Actuator> createActuator(TransactionCapsule transactionCapsul
actuatorList
.add(getActuatorByContract(contract, chainBaseManager, transactionCapsule));
} catch (IllegalAccessException | InstantiationException e) {
e.printStackTrace();
logger.error("Failed to create actuator for contract {}.", contract.getType(), e);
}
});
return actuatorList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ public long getEnergyForData(byte[] data) {
if (data == null) {
return 15;
}
return 15L + (data.length + 31) / 32 * 3;
return 15L + (data.length + 31L) / 32 * 3;
}

@Override
Expand All @@ -534,7 +534,7 @@ public long getEnergyForData(byte[] data) {
if (data == null) {
return 60;
}
return 60L + (data.length + 31) / 32 * 12;
return 60L + (data.length + 31L) / 32 * 12;
}

@Override
Expand All @@ -561,7 +561,7 @@ public long getEnergyForData(byte[] data) {
if (data == null) {
return 600;
}
return 600L + (data.length + 31) / 32 * 120;
return 600L + (data.length + 31L) / 32 * 120;
}

@Override
Expand Down Expand Up @@ -1043,7 +1043,7 @@ public static class ValidateMultiSign extends PrecompiledContract {

@Override
public long getEnergyForData(byte[] data) {
long cnt = (data.length / WORD_SIZE - 5) / 5;
long cnt = ((long) data.length / WORD_SIZE - ABI_HEADER_WORDS) / ABI_ITEM_WORDS;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The subtraction is performed before clamping the number of items, so malformed short calldata produces a negative count for ValidateMultiSign (for example, zero-length input yields -1) and a zero count for short BatchValidateSign input. In Program, any non-positive required energy bypasses the requiredEnergy > suppliedEnergy check, allowing malformed calls to proceed without being charged for the precompile. Clamp the count to a non-negative value or reject malformed calldata before calculating energy. [incorrect condition logic]

Severity Level: Major ⚠️
- ⚠️ Malformed multisignature calls bypass required-energy accounting.
- ❌ Legacy malformed decoding can fail during VM precompile execution.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java
**Line:** 1046:1046
**Comment:**
	*Incorrect Condition Logic: The subtraction is performed before clamping the number of items, so malformed short calldata produces a negative count for `ValidateMultiSign` (for example, zero-length input yields `-1`) and a zero count for short `BatchValidateSign` input. In `Program`, any non-positive required energy bypasses the `requiredEnergy > suppliedEnergy` check, allowing malformed calls to proceed without being charged for the precompile. Clamp the count to a non-negative value or reject malformed calldata before calculating energy.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

// one sign 1500, half of ecrecover
return cnt * ENGERYPERSIGN;
}
Expand Down Expand Up @@ -1136,7 +1136,7 @@ public static class BatchValidateSign extends PrecompiledContract {

@Override
public long getEnergyForData(byte[] data) {
long cnt = (data.length / WORD_SIZE - 5) / 6;
long cnt = ((long) data.length / WORD_SIZE - ABI_HEADER_WORDS) / ABI_ITEM_WORDS;
// one sign 1500, half of ecrecover
return cnt * ENGERYPERSIGN;
}
Expand Down
2 changes: 2 additions & 0 deletions actuator/src/main/java/org/tron/core/vm/program/Program.java
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,8 @@ private long transferFrozenV2BalanceToInheritor(byte[] ownerAddr, byte[] inherit
case TRON_POWER:
inheritorCapsule.addFrozenForTronPowerV2(freezeV2.getAmount());
break;
case UNRECOGNIZED:
break;
}
});

Expand Down
34 changes: 34 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,49 @@ subprojects {
errorprone "com.google.errorprone:error_prone_core:${errorproneVersion}"
errorprone rootProject.project(':errorprone')
}
// Keep test compilation free of Error Prone so existing test code does not block builds.
tasks.withType(JavaCompile).configureEach {
options.errorprone.enabled = false
}
tasks.named(sourceSets.main.compileJavaTaskName, JavaCompile).configure {
options.errorprone {
enabled = true
disableWarningsInGeneratedCode = true
disableAllChecks = true
excludedPaths = '.*/generated/.*'
errorproneArgs.addAll([
// Project-specific checks.
'-Xep:BigDecimalFloatingPointConstructor:ERROR',
'-Xep:SelfAssignment:ERROR',
'-Xep:StringCaseLocaleUsage:ERROR',
'-Xep:StringCaseLocaleUsageMethodRef:ERROR',
'-Xep:ForbidJavaLangMath:ERROR',
Comment on lines +140 to +144

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Run the new safety checks for x86 builds

These checks are added inside the existing javaVersion.isJava11Compatible() block, but this repository requires Java 8 when os.arch is x86/amd64, so production compilation on the Rocky Linux/JDK 8 path never applies any of the newly enabled checks. Consequently, x86 contributors can still compile floating-point BigDecimal constructors, self-assignments, and direct Math calls successfully; add a JDK 11+ verification/toolchain task that also runs for x86 sources rather than relying solely on the architecture-gated compileJava configuration.

Useful? React with 👍 / 👎.

'-Xep:ComparatorNeverReturnsZero:ERROR',

// High-signal checks.
'-Xep:ArrayEquals:ERROR',
'-Xep:ArrayHashCode:ERROR',
'-Xep:ArrayToString:ERROR',
'-Xep:ArraysAsListPrimitiveArray:ERROR',
'-Xep:BadShiftAmount:ERROR',
'-Xep:CollectionIncompatibleType:ERROR',
'-Xep:ConstantOverflow:ERROR',
'-Xep:EqualsHashCode:ERROR',
'-Xep:EqualsIncompatibleType:ERROR',
'-Xep:FloatingPointLiteralPrecision:ERROR',
'-Xep:GuardedBy:ERROR',
'-Xep:JavaTimeDefaultTimeZone:ERROR',
'-Xep:MathRoundIntLong:ERROR',
'-Xep:NonAtomicVolatileUpdate:ERROR',
'-Xep:ThreadJoinLoop:ERROR',
'-Xep:UnicodeEscape:ERROR',
'-Xep:XorPower:ERROR',

// Small production baseline fixed in this change.
'-Xep:IntLongMath:ERROR',
'-Xep:LockNotBeforeTry:ERROR',
'-Xep:MissingCasesInEnumSwitch:ERROR',
'-Xep:CatchAndPrintStackTrace:ERROR',
])
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,6 @@ public static PedersenHashCapsule uncommitted() throws ZksnarkException {
return compressCapsule;
}

public static void main(String[] args) {
try {
byte[] a =
ByteArray
.fromHexString("05655316a07e6ec8c9769af54ef98b30667bfb6302b32987d552227dae86a087");
byte[] b =
ByteArray
.fromHexString("06041357de59ba64959d1b60f93de24dfe5ea1e26ed9e8a73d35b225a1845ba7");

PedersenHash sa = PedersenHash.newBuilder().setContent(ByteString.copyFrom(a)).build();
PedersenHash sb = PedersenHash.newBuilder().setContent(ByteString.copyFrom(b)).build();

PedersenHash result = combine(sa, sb, 25).getInstance();
// 61a50a5540b4944da27cbd9b3d6ec39234ba229d2c461f4d719bc136573bf45b
System.out.println(ByteArray.toHexString(result.getContent().toByteArray()));
} catch (ZksnarkException e) {
e.printStackTrace();
}
}

public ByteString getContent() {
return this.pedersenHash.getContent();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,8 +786,10 @@ public String toString() {
getInstance().getRawData().getContractList().forEach(contract -> {
toStringBuff.append("[" + i + "] ").append("type: ").append(contract.getType())
.append("\n");
toStringBuff.append("from address=").append(getOwner(contract)).append("\n");
toStringBuff.append("to address=").append(getToAddress(contract)).append("\n");
toStringBuff.append("from address=")
.append(ByteArray.toHexString(getOwner(contract))).append("\n");
toStringBuff.append("to address=")
.append(ByteArray.toHexString(getToAddress(contract))).append("\n");
if (contract.getType().equals(ContractType.TransferContract)) {
TransferContract transferContract;
try {
Expand All @@ -796,7 +798,7 @@ public String toString() {
toStringBuff.append("transfer amount=").append(transferContract.getAmount())
.append("\n");
} catch (InvalidProtocolBufferException e) {
e.printStackTrace();
logger.debug("Failed to unpack transfer contract.", e);
}
} else if (contract.getType().equals(ContractType.TransferAssetContract)) {
TransferAssetContract transferAssetContract;
Expand All @@ -808,7 +810,7 @@ public String toString() {
toStringBuff.append("transfer amount=").append(transferAssetContract.getAmount())
.append("\n");
} catch (InvalidProtocolBufferException e) {
e.printStackTrace();
logger.debug("Failed to unpack transfer asset contract.", e);
}
}
if (this.transaction.getSignatureList().size() >= i.get() + 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public void consume(TransactionCapsule trx, TransactionTrace trace)
long maxCreateAccountTxSize = dynamicPropertiesStore.getMaxCreateAccountTxSize();
int signatureCount = trx.getInstance().getSignatureCount();
long createAccountBytesSize = trx.getInstance().toBuilder().clearRet()
.build().getSerializedSize() - (signatureCount * PER_SIGN_LENGTH);
.build().getSerializedSize() - ((long) signatureCount * PER_SIGN_LENGTH);
if (createAccountBytesSize > maxCreateAccountTxSize) {
throw new TooBigTransactionException(String.format(
"Too big new account transaction, TxId %s, the size is %d bytes, maxTxSize %d",
Expand Down Expand Up @@ -548,4 +548,3 @@ private boolean useFreeNet(AccountCapsule accountCapsule, long bytes, long now)

}


12 changes: 6 additions & 6 deletions chainbase/src/main/java/org/tron/core/store/AssetIssueStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static org.tron.common.utils.Commons.ASSET_ISSUE_COUNT_LIMIT_MAX;

import com.google.common.collect.Streams;
import com.google.protobuf.ByteString;
import java.util.Comparator;
import java.util.List;
import java.util.Map.Entry;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -46,12 +48,10 @@ private List<AssetIssueCapsule> getAssetIssuesPaginated(List<AssetIssueCapsule>
if (assetIssueList.size() <= offset) {
return null;
}
assetIssueList.sort((o1, o2) -> {
if (o1.getName() != o2.getName()) {
return o1.getName().toStringUtf8().compareTo(o2.getName().toStringUtf8());
}
return Long.compare(o1.getOrder(), o2.getOrder());
});
assetIssueList.sort(
Comparator.comparing(AssetIssueCapsule::getName,
ByteString.unsignedLexicographicalComparator())
.thenComparingLong(AssetIssueCapsule::getOrder));
limit = limit > ASSET_ISSUE_COUNT_LIMIT_MAX ? ASSET_ISSUE_COUNT_LIMIT_MAX : limit;
long end = offset + limit;
end = end > assetIssueList.size() ? assetIssueList.size() : end;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.tron.core.store;

import com.google.common.collect.Streams;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -31,9 +32,7 @@ public ExchangeCapsule get(byte[] key) throws ItemNotFoundException {
public List<ExchangeCapsule> getAllExchanges() {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.sorted(
(ExchangeCapsule a, ExchangeCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1
: -1)
.sorted(Comparator.comparingLong(ExchangeCapsule::getCreateTime).reversed())
.collect(Collectors.toList());
}
}
}
18 changes: 10 additions & 8 deletions chainbase/src/main/java/org/tron/core/store/ProposalStore.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.tron.core.store;

import com.google.common.collect.Streams;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -32,23 +33,24 @@ public ProposalCapsule get(byte[] key) throws ItemNotFoundException {
public List<ProposalCapsule> getAllProposals() {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.sorted(
(ProposalCapsule a, ProposalCapsule b) -> a.getCreateTime() <= b.getCreateTime() ? 1
: -1)
.sorted(Comparator.comparingLong(ProposalCapsule::getCreateTime).reversed())
.collect(Collectors.toList());
}

/**
* note: return in asc order by expired time
* Returns proposals in ascending expiration order, ties broken by descending proposal ID.
*
* <p>The descending-id tie-break preserves the execution order for equal-expiration proposals:
* live execution applies the highest id first and the lowest id last (final value), and the
* energy/bandwidth price-history loaders rebuild from the tail, so the lowest id must be last.
*/
public List<ProposalCapsule> getSpecifiedProposals(State state, long code) {
return Streams.stream(iterator())
.map(Map.Entry::getValue)
.filter(proposalCapsule -> proposalCapsule.getState().equals(state))
.filter(proposalCapsule -> proposalCapsule.getParameters().containsKey(code))
.sorted(
(ProposalCapsule a, ProposalCapsule b) -> a.getExpirationTime() > b.getExpirationTime()
? 1 : -1)
.sorted(Comparator.comparingLong(ProposalCapsule::getExpirationTime)
.thenComparing(Comparator.comparingLong(ProposalCapsule::getID).reversed()))
.collect(Collectors.toList());
}
}
}
2 changes: 1 addition & 1 deletion common/src/main/java/org/tron/common/utils/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public static int readData(String filePath, char[] buf) {
try (BufferedReader bufRead = new BufferedReader(new FileReader(file))) {
len = bufRead.read(buf, 0, buf.length);
} catch (IOException ex) {
ex.printStackTrace();
logger.warn("Failed to read data from file.", ex);
return 0;
}
return len;
Expand Down
17 changes: 17 additions & 0 deletions errorprone/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,29 @@ if (!JavaVersion.current().isJava11Compatible()) {
// ErrorProne core requires JDK 11+; skip this module on JDK 8
tasks.withType(JavaCompile).configureEach { enabled = false }
tasks.withType(Jar).configureEach { enabled = false }
tasks.withType(Test).configureEach { enabled = false }
} else {
dependencies {
compileOnly "com.google.errorprone:error_prone_annotations:${errorproneVersion}"
compileOnly "com.google.errorprone:error_prone_check_api:${errorproneVersion}"
compileOnly "com.google.errorprone:error_prone_core:${errorproneVersion}"
compileOnly "com.google.auto.service:auto-service:1.1.1"
annotationProcessor "com.google.auto.service:auto-service:1.1.1"
testImplementation "com.google.errorprone:error_prone_test_helpers:${errorproneVersion}"
}

tasks.withType(Test).configureEach {
jvmArgs(
'--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED',
'--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED',
'--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED',
'--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED'
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package errorprone;

import com.google.auto.service.AutoService;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.NewClassTree;
import com.sun.tools.javac.code.Symbol;
import java.util.List;

/**
* Prevents constructing {@link java.math.BigDecimal} from binary floating-point values.
*
* <p>This checks the resolved constructor signature, so it also catches {@link Double} and
* {@link Float} arguments that javac unboxes to the {@code double} constructor.
*/
@AutoService(BugChecker.class)
@BugPattern(
name = "BigDecimalFloatingPointConstructor",
summary = "Do not construct BigDecimal from a floating-point value. Use a decimal String, "
+ "for example new BigDecimal(\"0.0001\").",
severity = BugPattern.SeverityLevel.ERROR
)
public class BigDecimalFloatingPointConstructor extends BugChecker
implements BugChecker.NewClassTreeMatcher {

private static final String BIG_DECIMAL = "java.math.BigDecimal";

@Override
public Description matchNewClass(NewClassTree tree, VisitorState state) {
Symbol symbol = ASTHelpers.getSymbol(tree);
if (!(symbol instanceof Symbol.MethodSymbol)) {
return Description.NO_MATCH;
}

Symbol.MethodSymbol constructor = (Symbol.MethodSymbol) symbol;
if (!constructor.owner.getQualifiedName().contentEquals(BIG_DECIMAL)) {
return Description.NO_MATCH;
}

List<Symbol.VarSymbol> parameters = constructor.getParameters();
if (parameters.isEmpty()
|| !ASTHelpers.isSameType(parameters.get(0).type, state.getSymtab().doubleType, state)) {
return Description.NO_MATCH;
}

return describeMatch(tree);
}
}
Loading
Loading