feat(detectors): Add SuspiciousLoop (#206)

Closes #171

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - `UnprotectedCall` detector: PR [#235](https://github.com/nowarp/misti/pull/235)
+- `SuspiciousLoop` detector: PR [#206](https://github.com/nowarp/misti/pull/206)
 - File-scoped CFG dumps: Issue [#241](https://github.com/nowarp/misti/issues/241)
 
 ### Fixed

README.md

@@ -2,7 +2,7 @@
 Misti is a static analysis tool designed for smart contracts on the [TON blockchain](https://ton.org/).
 
 #### Language Support
-- [Tact](https://tact-lang.org/): 28 detectors [are available](https://nowarp.io/tools/misti/docs/next/detectors)
+- [Tact](https://tact-lang.org/): 30 detectors [are available](https://nowarp.io/tools/misti/docs/next/detectors)
 - [FunC](https://docs.ton.org/develop/func/overview) support is [planned](https://github.com/nowarp/misti/issues/56) by the end of the year
 
 #### Features

src/detectors/builtin/suspiciousLoop.ts

@@ -0,0 +1,103 @@
+import { CompilationUnit } from "../../internals/ir";
+import {
+  foldStatements,
+  evalsToValue,
+  evalsToPredicate,
+} from "../../internals/tact";
+import { MistiTactWarning, Severity } from "../../internals/warnings";
+import { AstDetector } from "../detector";
+import {
+  AstStatement,
+  AstExpression,
+} from "@tact-lang/compiler/dist/grammar/ast";
+
+/**
+ * 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 (10_001) { // Bad: High iteration count
+ *     // ...
+ * }
+ *
+ * while (true) { // Bad: Unbounded condition
+ *     // ...
+ * }
+ * ```
+ */
+export class SuspiciousLoop extends AstDetector {
+  severity = Severity.MEDIUM;
+
+  async check(cu: CompilationUnit): Promise<MistiTactWarning[]> {
+    return Array.from(cu.ast.getProgramEntries()).reduce((acc, node) => {
+      return acc.concat(
+        ...foldStatements(
+          node,
+          (acc, stmt) => {
+            return acc.concat(this.analyzeLoopStatement(stmt));
+          },
+          acc,
+        ),
+      );
+    }, [] as MistiTactWarning[]);
+  }
+
+  /**
+   * Analyzes a loop statement to determine if it contains a suspicious condition.
+   */
+  private analyzeLoopStatement(stmt: AstStatement): MistiTactWarning[] {
+    if (
+      stmt.kind === "statement_repeat" &&
+      evalsToPredicate(stmt.iterations, (v) => v > 10_000n)
+    ) {
+      return [
+        this.makeWarning("Potential high-cost loop", stmt.iterations.loc, {
+          suggestion: "Avoid excessive iterations in loops",
+        }),
+      ];
+    }
+    if (stmt.kind === "statement_while") {
+      let warnings: MistiTactWarning[] = [];
+      warnings = warnings.concat(this.checkTrueCondition(stmt.condition));
+      if (warnings.length === 0 && stmt.statements.length > 0) {
+        warnings = warnings.concat(this.checkFalseCondition(stmt.condition));
+      }
+      return warnings;
+    }
+    if (stmt.kind === "statement_until") {
+      let warnings: MistiTactWarning[] = [];
+      warnings = warnings.concat(this.checkTrueCondition(stmt.condition));
+      return warnings;
+    }
+    return [];
+  }
+
+  private checkFalseCondition(expr: AstExpression): MistiTactWarning[] {
+    if (evalsToValue(expr, "boolean", false)) {
+      return [
+        this.makeWarning("Loop condition is always false", expr.loc, {
+          suggestion:
+            "The condition is always false; the body will never execute",
+        }),
+      ];
+    }
+    return [];
+  }
+
+  private checkTrueCondition(expr: AstExpression): MistiTactWarning[] {
+    if (evalsToValue(expr, "boolean", true)) {
+      return [
+        this.makeWarning("Infinite loop detected", expr.loc, {
+          suggestion: "Avoid unbounded conditions in loops",
+        }),
+      ];
+    }
+    return [];
+  }
+}

src/detectors/detector.ts

@@ -406,6 +406,13 @@ export const BuiltInDetectors: Record<string, DetectorEntry> = {
       ),
     enabledByDefault: true,
   },
+  SuspiciousLoop: {
+    loader: (ctx: MistiContext) =>
+      import("./builtin/suspiciousLoop").then(
+        (module) => new module.SuspiciousLoop(ctx),
+      ),
+    enabledByDefault: true,
+  },
 };
 
 /**

test/README.md

@@ -1,5 +1,3 @@
-Here's the updated text with the grammar fixed:
-
 # Tests
 
 ## Structure

test/detector.spec.ts

@@ -13,7 +13,8 @@ describe("Common detectors functionality", () => {
   it(`should generate valid JSON output for ${CONTRACT_NAME}`, async () => {
     const filePath = ABSOLUTE_PATH;
     const output = await executeMisti([
-      "--all-detectors",
+      "--enabled-detectors",
+      "NeverAccessedVariables",
       "--no-colors",
       "--output-format",
       "json",

test/detectors/NeverAccessedVariables.tact

@@ -77,4 +77,3 @@ contract TestContract2 {
         let _test: Int = 123; // suppressed: no warning
     }
 }
-

test/detectors/SuspiciousLoop.expected.out

@@ -0,0 +1,44 @@
+[MEDIUM] SuspiciousLoop: Infinite loop detected
+test/detectors/SuspiciousLoop.tact:10:16:
+   9 |         let i: Int = 0;
+> 10 |         while (true) { i = i + 1; } // Bad
+                      ^
+  11 |         do { i = i + 1; } until (true); // Bad
+Help: Avoid unbounded conditions in loops
+See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop
+
+[MEDIUM] SuspiciousLoop: Infinite loop detected
+test/detectors/SuspiciousLoop.tact:11:34:
+  10 |         while (true) { i = i + 1; } // Bad
+> 11 |         do { i = i + 1; } until (true); // Bad
+                                        ^
+  12 |         do { i = i + 1; } until (TRUE); // TODO: Unsupported in consteval
+Help: Avoid unbounded conditions in loops
+See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop
+
+[MEDIUM] SuspiciousLoop: Potential high-cost loop
+test/detectors/SuspiciousLoop.tact:16:17:
+  15 |     fun testRepeatHighCount() {
+> 16 |         repeat (10_001) { let x = 1; } // Bad
+                       ^
+  17 |         repeat (A + B) { let x = 1; } // TODO: Unsupported in consteval
+Help: Avoid excessive iterations in loops
+See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop
+
+[MEDIUM] SuspiciousLoop: Potential high-cost loop
+test/detectors/SuspiciousLoop.tact:24:21:
+  23 |         while (i < 10) {
+> 24 |             repeat (1_000_000) { i = i + 1; } // Bad
+                           ^
+  25 |         }
+Help: Avoid excessive iterations in loops
+See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop
+
+[MEDIUM] SuspiciousLoop: Loop condition is always false
+test/detectors/SuspiciousLoop.tact:30:16:
+  29 |         let a: Int = 0;
+> 30 |         while (false) { a = 1; } // Bad
+                      ^
+  31 |         while (false) { } // OK: Empty body
+Help: The condition is always false; the body will never execute
+See: https://nowarp.io/tools/misti/docs/detectors/SuspiciousLoop

test/detectors/SuspiciousLoop.tact

@@ -0,0 +1,36 @@
+const TRUE: Bool = true;
+const FALSE: Bool = false;
+const A: Int = 1000;
+const B: Int = 10000;
+
+contract TestSuspiciousLoops {
+
+    fun testWhileInfinite() {
+        let i: Int = 0;
+        while (true) { i = i + 1; } // Bad
+        do { i = i + 1; } until (true); // Bad
+        do { i = i + 1; } until (TRUE); // TODO: Unsupported in consteval
+    }
+
+    fun testRepeatHighCount() {
+        repeat (10_001) { let x = 1; } // Bad
+        repeat (A + B) { let x = 1; } // TODO: Unsupported in consteval
+        repeat (A) { let x = 1; } // OK
+    }
+
+    fun testNestedLoops() {
+        let i: Int = 0;
+        while (i < 10) {
+            repeat (1_000_000) { i = i + 1; } // Bad
+        }
+    }
+
+    fun testLoop() {
+        let a: Int = 0;
+        while (false) { a = 1; } // Bad
+        while (false) { } // OK: Empty body
+        do { a = 1; } until (false); // OK: Until is allowed
+        do {} until (false); // OK: Until is allowed
+        while (TRUE && FALSE) { a = 1; } // TODO: Unsupported in consteval
+    }
+}