Skip to content
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.

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

Suspicious Loop Detector #206

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
132 changes: 132 additions & 0 deletions src/detectors/builtin/suspiciousLoop.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import { CompilationUnit } from "../../internals/ir";
import {
foldStatements,
foldExpressions,
evalExpr,
} from "../../internals/tact";
import { MistiTactWarning, Severity } from "../../internals/warnings";
import { ASTDetector } from "../detector";
import {
AstStatement,
AstExpression,
} from "@tact-lang/compiler/dist/grammar/ast";

/**
* SuspiciousLoop Detector
*
* An optional detector that identifies potentially problematic loops, such as those
* with unbounded conditions or excessive iteration counts.
*
* ## Why is it bad?
* Loops with always-true conditions or massive iteration limits can lead to high
* gas consumption and even denial of service (DoS) issues. By flagging these loops,
* this detector aids auditors in catching potential performance or security risks.
*
* ## Example
* ```tact
* repeat (1_000_001) { // Highlighted by detector as high iteration count
* // ...
* }
*
* while (true) { // Highlighted as unbounded condition
* // ...
* }
* ```
*/
export class SuspiciousLoop extends ASTDetector {
severity = Severity.HIGH;

async check(cu: CompilationUnit): Promise<MistiTactWarning[]> {
const processedLoopIds = new Set<number>();
return Array.from(cu.ast.getProgramEntries()).reduce((acc, node) => {
return acc.concat(
...foldStatements(
node,
(acc, stmt) => {
return acc.concat(
this.analyzeLoopStatement(stmt, processedLoopIds),
);
},
acc,
{ flatStmts: true },
),
);
}, [] as MistiTactWarning[]);
}

/**
* Analyzes a loop statement to determine if it contains a suspicious condition.
* @param stmt - The statement to analyze.
* @param processedLoopIds - A set of loop IDs already processed.
* @returns An array of MistiTactWarning objects if a suspicious loop is detected.
*/
private analyzeLoopStatement(
stmt: AstStatement,
processedLoopIds: Set<number>,
): MistiTactWarning[] {
if (processedLoopIds.has(stmt.id)) {
return [];
}
if (this.isLoop(stmt)) {
processedLoopIds.add(stmt.id);
return foldExpressions(
stmt,
(acc, expr) => {
if (this.isSuspiciousCondition(expr)) {
acc.push(
this.makeWarning(
"Potential unbounded or high-cost loop",
stmt.loc,
{
suggestion:
"Avoid excessive iterations or unbounded conditions in loops",
},
),
);
}
return acc;
},
[] as MistiTactWarning[],
);
}
return [];
}

/**
* Checks if an expression is a suspicious condition, indicating an unbounded
* loop or excessive iteration.
* @param expr - The expression to evaluate.
* @returns True if the expression is suspicious, otherwise false.
*/

private isSuspiciousCondition(expr: AstExpression): boolean {
// Try evaluating the expression as a constant.
const result = evalExpr(expr);
if (result !== undefined) {
if (typeof result === "boolean" && result === true) {
// Unbounded loop detected
return true;
}
// Check if the result is a large constant for repeat counts.
const threshold = 100_000;
if (typeof result === "bigint" && result > BigInt(threshold)) {
return true;
}
}
return false;
}

/**
* Determines if a statement is a loop.
* @param stmt - The statement to evaluate.
* @returns True if the statement represents a loop, otherwise false.
*/
private isLoop(stmt: AstStatement): boolean {
return (
stmt.kind === "statement_while" ||
stmt.kind === "statement_repeat" ||
stmt.kind === "statement_until" ||
stmt.kind === "statement_foreach"
);
}
}
7 changes: 7 additions & 0 deletions src/detectors/detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,13 @@ export const BuiltInDetectors: Record<string, DetectorEntry> = {
),
enabledByDefault: true,
},
SuspiciousLoop: {
loader: (ctx: MistiContext) =>
import("./builtin/suspiciousLoop").then(
(module) => new module.SuspiciousLoop(ctx),
),
enabledByDefault: false,
},
};

/**
Expand Down
35 changes: 35 additions & 0 deletions test/detectors/SuspiciousLoop.expected.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
[HIGH] SuspiciousLoop: Potential unbounded or high-cost loop
test/detectors/SuspiciousLoop.tact:5:9:
4 | let i: Int = 0;
> 5 | while (true) { // This should be flagged for an unbounded condition
^
6 | i = i + 1;
Help: Avoid excessive iterations or unbounded conditions in loops
See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop

[HIGH] SuspiciousLoop: Potential unbounded or high-cost loop
test/detectors/SuspiciousLoop.tact:11:9:
10 | fun testRepeatHighCount() {
> 11 | repeat (1_000_001) { // This should be flagged for excessive iteration
^
12 | let x = 1;
Help: Avoid excessive iterations or unbounded conditions in loops
See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop

[HIGH] SuspiciousLoop: Potential unbounded or high-cost loop
test/detectors/SuspiciousLoop.tact:18:9:
17 | let i: Int = 0;
> 18 | while (i < 1_000_000) { // This should be flagged for large constant comparison
^
19 | i = i + 1;
Help: Avoid excessive iterations or unbounded conditions in loops
See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop

[HIGH] SuspiciousLoop: Potential unbounded or high-cost loop
test/detectors/SuspiciousLoop.tact:25:9:
24 | let i: Int = 0;
> 25 | while (i < 10) {
^
26 | repeat (1_000_000) { // Should be flagged within nested loop
Help: Avoid excessive iterations or unbounded conditions in loops
See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop
31 changes: 31 additions & 0 deletions test/detectors/SuspiciousLoop.tact
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
contract TestSuspiciousLoops {

fun testWhileInfinite() {
let i: Int = 0;
while (true) { // This should be flagged for an unbounded condition
i = i + 1;
}
}

fun testRepeatHighCount() {
repeat (1_000_001) { // This should be flagged for excessive iteration
let x = 1;
}
}

fun testWhileLargeLimit() {
let i: Int = 0;
while (i < 1_000_000) { // This should be flagged for large constant comparison
i = i + 1;
}
}

fun testNestedLoops() {
let i: Int = 0;
while (i < 10) {
repeat (1_000_000) { // Should be flagged within nested loop
i = i + 1;
}
}
}
}
Loading