Suspicious Loop Detector

src/detectors/builtin/suspiciousLoop.ts

@@ -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"
+    );
+  }
+}

src/detectors/detector.ts

@@ -205,6 +205,13 @@ interface DetectorEntry {
  * A mapping of detector names to their respective loader functions and default enablement status.
  */
 export const BuiltInDetectors: Record<string, DetectorEntry> = {
+  NeverAccessedVariables: {
+    loader: (ctx: MistiContext) =>
+      import("./builtin/neverAccessedVariables").then(
+        (module) => new module.NeverAccessedVariables(ctx),
+      ),
+    enabledByDefault: true,
+  },
   DivideBeforeMultiply: {
     loader: (ctx: MistiContext) =>
       import("./builtin/divideBeforeMultiply").then(
@@ -219,13 +226,6 @@ export const BuiltInDetectors: Record<string, DetectorEntry> = {
       ),
     enabledByDefault: true,
   },
-  NeverAccessedVariables: {
-    loader: (ctx: MistiContext) =>
-      import("./builtin/neverAccessedVariables").then(
-        (module) => new module.NeverAccessedVariables(ctx),
-      ),
-    enabledByDefault: true,
-  },
   UnboundLoop: {
     loader: (ctx: MistiContext) =>
       import("./builtin/unboundLoop").then(
@@ -399,6 +399,13 @@ export const BuiltInDetectors: Record<string, DetectorEntry> = {
       ),
     enabledByDefault: true,
   },
+  SuspiciousLoop: {
+    loader: (ctx: MistiContext) =>
+      import("./builtin/suspiciousLoop").then(
+        (module) => new module.SuspiciousLoop(ctx),
+      ),
+    enabledByDefault: false,
+  },
 };
 
 /**

test/detectors/NeverAccessedVariables.tact

@@ -5,10 +5,10 @@ fun test1(): Int {
     }
     return 42;
 }
-
-fun test2() {
-    while(true) { let a: Int = 0; }
-}
+// suppress SuspiciousLoop
+//fun test2() {
+//    while(true) { let a: Int = 0; }
+//}
 
 fun test3(): Int {
     let a: Int = 20; // read-only variable

test/detectors/SuspiciousLoop.expected.out

@@ -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

test/detectors/SuspiciousLoop.tact

@@ -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;
+            }
+        }
+    }
+}