From 601e15edc056557ef3397e8c293921b026f3d07a Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Mon, 28 Oct 2024 13:25:26 +0000 Subject: [PATCH 1/8] =?UTF-8?q?chore(README):=20Souffl=C3=A9=20installatio?= =?UTF-8?q?n=20is=20optional?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b5dc7d41..827be752 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Misti is a static analysis tool designed for smart contracts on the [TON blockch - **Custom Detectors**: Create [custom detectors](https://nowarp.io/tools/misti/docs/hacking/custom-detector) to solve specific problems in your code or to provide a thorough security review if you are an auditor. ## Getting Started -1. Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install). +1. (optional) Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install). 2. Misti is distributed via npm and should be added to your Tact project [in the same way](https://github.com/tact-lang/tact?tab=readme-ov-file#installation) as Tact itself: ```bash From 3c31e7ff82df9c356c27ff221ed804270739e1b6 Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Mon, 28 Oct 2024 22:33:24 -0400 Subject: [PATCH 2/8] feat(driver): Support suppression annotations (#203) Closes #43 --- CHANGELOG.md | 1 + src/cli/driver.ts | 28 +++++++++- src/detectors/detector.ts | 15 ++--- src/index.ts | 1 + src/internals/annotation.ts | 61 +++++++++++++++++++++ src/internals/tact/util.ts | 17 ++++++ src/internals/warnings.ts | 33 ++++++----- test/detectors/AsmIsUsed.tact | 4 ++ test/detectors/FieldDoubleInit.expected.out | 10 ++-- test/detectors/FieldDoubleInit.tact | 7 ++- 10 files changed, 141 insertions(+), 36 deletions(-) create mode 100644 src/internals/annotation.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bfa1904..228a8e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `CellOverflow` detector: PR [#177](https://github.com/nowarp/misti/pull/177) - `UnboundMap` detector: Issue [#50](https://github.com/nowarp/misti/issues/50) - `UnusedExpressionResult` detector: PR [#190](https://github.com/nowarp/misti/pull/190) +- Warning suppressions: PR [#203](https://github.com/nowarp/misti/pull/203) - `--list-detectors` CLI option: PR [#192](https://github.com/nowarp/misti/pull/192) - Import Graph: PR [#180](https://github.com/nowarp/misti/pull/180) - Leverage `ImportGraph` to resolve entry points: PR [#194](https://github.com/nowarp/misti/pull/194) diff --git a/src/cli/driver.ts b/src/cli/driver.ts index b08e7c59..b5b69b68 100644 --- a/src/cli/driver.ts +++ b/src/cli/driver.ts @@ -511,11 +511,37 @@ export class Driver { } /** - * Filters out the warnings suppressed in the configuration files. + * Filters out the suppressed warnings. * Mutates the input map removing suppressed warnings. */ private filterSuppressedWarnings( warnings: Map, + ): void { + this.filterSuppressedInAnnotations(warnings); + this.filterSuppressedInConfig(warnings); + } + + /** + * Filters out the warnings suppressed in the code annotations. + * Mutates the input map removing suppressed warnings. + */ + private filterSuppressedInAnnotations( + warnings: Map, + ): void { + warnings.forEach((projectWarnings, projectName) => { + const filteredWarnings = projectWarnings.filter( + (warning) => !warning.isSuppressed(), + ); + warnings.set(projectName, filteredWarnings); + }); + } + + /** + * Filters out the warnings suppressed in the configuration files. + * Mutates the input map removing suppressed warnings. + */ + private filterSuppressedInConfig( + warnings: Map, ): void { this.ctx.config.suppressions.forEach((suppression) => { let suppressionUsed = false; diff --git a/src/detectors/detector.ts b/src/detectors/detector.ts index 2fe575eb..6c2780a5 100644 --- a/src/detectors/detector.ts +++ b/src/detectors/detector.ts @@ -104,17 +104,10 @@ export abstract class Detector { suggestion: string; }> = {}, ): MistiTactWarning { - return MistiTactWarning.make( - this.ctx, - this.id, - description, - this.severity, - loc, - { - ...data, - docURL: hasBuiltInDetector(this.id) ? makeDocURL(this.id) : undefined, - }, - ); + return MistiTactWarning.make(this.id, description, this.severity, loc, { + ...data, + docURL: hasBuiltInDetector(this.id) ? makeDocURL(this.id) : undefined, + }); } } diff --git a/src/index.ts b/src/index.ts index b16dadd4..9e861ab0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,6 +2,7 @@ export * from "./version"; export * from "./detectors/detector"; export * from "./tools/"; export * from "./internals/warnings"; +export * from "./internals/annotation"; export * from "./internals/transfer"; export * from "./internals/tact"; export * from "./internals/logger"; diff --git a/src/internals/annotation.ts b/src/internals/annotation.ts new file mode 100644 index 00000000..94bb9ebb --- /dev/null +++ b/src/internals/annotation.ts @@ -0,0 +1,61 @@ +/** + * Represents annotations in the comments processed by Misti. + * + * @packageDocumentation + */ + +import { srcInfoToString } from "./tact"; +import { SrcInfo } from "@tact-lang/compiler/dist/grammar/grammar"; + +/** + * Represents a Misti annotation. + */ +export type MistiAnnotation = { + kind: "suppress"; + detectors: string[]; +}; + +/** + * The marker used to identify Misti suppress annotations. + * Syntax: // @misti:suppress Detector1,Detector2 + */ +export const SUPPRESS_MARKER = "@misti:suppress"; + +/** + * Retrieves the Misti annotation from the current source location if present. + * + * These can be single or multi-line comments on the current or previous line + * annotated with SUPPRESS_MARKER. + */ +export function getMistiAnnotation(loc: SrcInfo): MistiAnnotation | null { + const lines = srcInfoToString(loc).split("\n"); + const currentLineIndex = lines.findIndex((line) => + line.trim().startsWith(">"), + ); + if (currentLineIndex <= 0) return null; + const previousLine = lines[currentLineIndex - 1]; + const previousLineCode = getCodeFromLine(previousLine); + const annotationPattern = new RegExp( + `^\\s*(\\/\\/|\\/\\*)\\s*${SUPPRESS_MARKER}\\s+([\\w,]+)\\s*`, + ); + + const match = previousLineCode.match(annotationPattern); + if (match) { + const detectors = match[2].split(",").map((detector) => detector.trim()); + return { + kind: "suppress", + detectors, + }; + } + + return null; +} + +function getCodeFromLine(line: string): string { + const pipeIndex = line.indexOf("|"); + if (pipeIndex !== -1) { + return line.substring(pipeIndex + 1).trim(); + } else { + return line.trim(); + } +} diff --git a/src/internals/tact/util.ts b/src/internals/tact/util.ts index f2badef4..4188b4d0 100644 --- a/src/internals/tact/util.ts +++ b/src/internals/tact/util.ts @@ -347,3 +347,20 @@ export function collectConditions( } return conditions; } + +/** + * Converts SrcInfo to the string representation shown to the user. + */ +export function srcInfoToString(loc: SrcInfo): string { + const lc = loc.interval.getLineAndColumn() as { + lineNum: number; + colNum: number; + }; + const lcStr = `${lc}`; + const lcLines = lcStr.split("\n"); + lcLines.shift(); + const shownPath = loc.file + ? path.relative(process.cwd(), loc.file) + ":" + : ""; + return `${shownPath}${lc.lineNum}:${lc.colNum}:\n${lcLines.join("\n")}`; +} diff --git a/src/internals/warnings.ts b/src/internals/warnings.ts index b7c57d37..8a2156ea 100644 --- a/src/internals/warnings.ts +++ b/src/internals/warnings.ts @@ -1,7 +1,7 @@ -import { MistiContext } from "./context"; +import { getMistiAnnotation } from "./annotation"; import { InternalException } from "./exceptions"; +import { srcInfoToString } from "./tact"; import { SrcInfo } from "@tact-lang/compiler/dist/grammar/ast"; -import * as path from "path"; /** * Enumerates the levels of severity that can be assigned to detected findings. @@ -99,7 +99,6 @@ export class MistiTactWarning { * @returns A new MistiTactWarning containing the warning message and source code reference. */ public static make( - ctx: MistiContext, detectorId: string, description: string, severity: Severity, @@ -118,19 +117,6 @@ export class MistiTactWarning { docURL = undefined, suggestion = undefined, } = data; - const pos = loc.file - ? (() => { - const lc = loc.interval.getLineAndColumn() as { - lineNum: number; - colNum: number; - }; - const lcStr = `${lc}`; - const lcLines = lcStr.split("\n"); - lcLines.shift(); - const shownPath = path.relative(process.cwd(), loc.file); - return `${shownPath}:${lc.lineNum}:${lc.colNum}:\n${lcLines.join("\n")}`; - })() - : ""; const extraDescriptionStr = extraDescription === undefined ? "" : extraDescription + "\n"; const suggestionStr = suggestion === undefined ? "" : `Help: ${suggestion}`; @@ -138,13 +124,26 @@ export class MistiTactWarning { const msg = [ description, "\n", - pos, + srcInfoToString(loc), extraDescriptionStr, suggestionStr, docURLStr, ].join(""); return new MistiTactWarning(detectorId, msg, loc, severity); } + + /** + * Checks whether this warning is suppressing using a Misti annotation. + */ + public isSuppressed(): boolean { + const annotation = getMistiAnnotation(this.loc); + if (annotation && annotation.kind === "suppress") { + return ( + annotation.detectors.find((d) => d === this.detectorId) !== undefined + ); + } + return false; + } } /** diff --git a/test/detectors/AsmIsUsed.tact b/test/detectors/AsmIsUsed.tact index 2a73e50f..41f7d9b7 100644 --- a/test/detectors/AsmIsUsed.tact +++ b/test/detectors/AsmIsUsed.tact @@ -1 +1,5 @@ asm(-> 1 0) extends mutates fun loadRefEx(self: Slice): Cell { LDREF } + +// OK: Suppressed +// @misti:suppress AsmIsUsed +asm(-> 1 0) extends mutates fun loadRefExSuppressed(self: Slice): Cell { LDREF } diff --git a/test/detectors/FieldDoubleInit.expected.out b/test/detectors/FieldDoubleInit.expected.out index 9c331e77..02f16737 100644 --- a/test/detectors/FieldDoubleInit.expected.out +++ b/test/detectors/FieldDoubleInit.expected.out @@ -1,8 +1,8 @@ -[MEDIUM] FieldDoubleInit: Field a is initialized twice -test/detectors/FieldDoubleInit.tact:4:9: - 3 | init(x: Int) { -> 4 | self.a = x; // Should be highlighted +[MEDIUM] FieldDoubleInit: Field a1 is initialized twice +test/detectors/FieldDoubleInit.tact:5:9: + 4 | init(x: Int) { +> 5 | self.a1 = x; // Should be highlighted ^ - 5 | } + 6 | // @misti:suppress FieldDoubleInit Help: Consider initializing the field only in its declaration or in the `init` function See: https://nowarp.io/tools/misti/docs/detectors/FieldDoubleInit \ No newline at end of file diff --git a/test/detectors/FieldDoubleInit.tact b/test/detectors/FieldDoubleInit.tact index 8ef274cb..85c7669b 100644 --- a/test/detectors/FieldDoubleInit.tact +++ b/test/detectors/FieldDoubleInit.tact @@ -1,7 +1,10 @@ contract Test1 { - a: Int = 0; + a1: Int = 0; + a2: Int = 0; init(x: Int) { - self.a = x; // Should be highlighted + self.a1 = x; // Should be highlighted + // @misti:suppress FieldDoubleInit + self.a2 = x; // OK: Suppressed } } From 74a89c4186e4a2664a5b2caa3b45b9989d00d717 Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 12:25:47 +0000 Subject: [PATCH 3/8] fix(sendInLoop): Stack overflow on large contracts --- src/detectors/builtin/sendInLoop.ts | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/detectors/builtin/sendInLoop.ts b/src/detectors/builtin/sendInLoop.ts index fe458c5b..6945cc04 100644 --- a/src/detectors/builtin/sendInLoop.ts +++ b/src/detectors/builtin/sendInLoop.ts @@ -1,5 +1,9 @@ import { CompilationUnit } from "../../internals/ir"; -import { foldStatements, foldExpressions, isSelf } from "../../internals/tact"; +import { + forEachStatement, + foldExpressions, + isSelf, +} from "../../internals/tact"; import { MistiTactWarning, Severity } from "../../internals/warnings"; import { ASTDetector } from "../detector"; import { @@ -38,18 +42,16 @@ export class SendInLoop extends ASTDetector { async check(cu: CompilationUnit): Promise { const processedLoopIds = new Set(); - return Array.from(cu.ast.getProgramEntries()).reduce((acc, node) => { - return acc.concat( - ...foldStatements( - node, - (acc, stmt) => { - return acc.concat(this.analyzeStatement(stmt, processedLoopIds)); - }, - acc, - { flatStmts: true }, - ), - ); - }, [] as MistiTactWarning[]); + const allWarnings: MistiTactWarning[] = []; + + Array.from(cu.ast.getProgramEntries()).forEach((node) => { + forEachStatement(node, (stmt) => { + const warnings = this.analyzeStatement(stmt, processedLoopIds); + allWarnings.push(...warnings); + }); + }); + + return allWarnings; } private analyzeStatement( From ba2a04ee09b05337a225b148302ed25a40b4033f Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 15:17:31 +0000 Subject: [PATCH 4/8] chore: Update readme --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 827be752..5fad638f 100644 --- a/README.md +++ b/README.md @@ -1,27 +1,27 @@ # Misti Logo Misti Misti is a static analysis tool designed for smart contracts on the [TON blockchain](https://ton.org/). -### Language Support +#### Language Support - [Tact](https://tact-lang.org/): 25 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 -### Use Cases -- **Detect Code Issues**: Identify and fix potential [security flaws and code problems](https://nowarp.io/tools/misti/docs/detectors) early in the development cycle. -- **Streamline Development**: +#### Features +- **Code Analysis**: Identify and fix potential [security flaws and code problems](https://nowarp.io/tools/misti/docs/detectors) early in the development cycle. +- **CI/CD Integration**: [Integrate](https://nowarp.io/tools/misti/docs/tutorial/ci-cd) Misti into your CI/CD pipeline to ensure continuous code quality checks. - **Custom Detectors**: Create [custom detectors](https://nowarp.io/tools/misti/docs/hacking/custom-detector) to solve specific problems in your code or to provide a thorough security review if you are an auditor. ## Getting Started 1. (optional) Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install). -2. Misti is distributed via npm and should be added to your Tact project [in the same way](https://github.com/tact-lang/tact?tab=readme-ov-file#installation) as Tact itself: +2. Install Misti: ```bash -yarn add @nowarp/misti +npm install -g @nowarp/misti ``` -3. Run Misti by specifying a Tact project configuration: +3. Run Misti by specifying a Tact contract, project config, or directory to check: ```bash -npx misti path/to/your/tact.config.json +misti path/to/contracts ``` See [Misti Configuration](https://nowarp.io/tools/misti/docs/tutorial/getting-started/) for available options, or [Developing Misti](https://nowarp.io/tools/misti/docs/next/hacking/developing-misti) for advanced instructions. Blueprint users should refer to the [appropriate documentation page](https://nowarp.io/tools/misti/docs/tutorial/blueprint). From 6a871bc9cdd936775cc123faab1922863fbe2e29 Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 15:19:31 +0000 Subject: [PATCH 5/8] chore: Update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5fad638f..db4bbc73 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Misti is a static analysis tool designed for smart contracts on the [TON blockch - **Custom Detectors**: Create [custom detectors](https://nowarp.io/tools/misti/docs/hacking/custom-detector) to solve specific problems in your code or to provide a thorough security review if you are an auditor. ## Getting Started -1. (optional) Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install). +1. *(optional)* Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install) to enable more built-in functionality. 2. Install Misti: ```bash From f251900147af4efc2ccda7e9473a7cb413d9aa14 Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 15:20:09 +0000 Subject: [PATCH 6/8] chore: Update readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index db4bbc73..669809f3 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Misti is a static analysis tool designed for smart contracts on the [TON blockch - **Custom Detectors**: Create [custom detectors](https://nowarp.io/tools/misti/docs/hacking/custom-detector) to solve specific problems in your code or to provide a thorough security review if you are an auditor. ## Getting Started -1. *(optional)* Install Soufflé according to [the official installation instruction](https://souffle-lang.github.io/install) to enable more built-in functionality. +1. *(optional)* [Install Soufflé](https://souffle-lang.github.io/install) to enable more built-in functionality. 2. Install Misti: ```bash From 49c942d14612a1f8293f55517d737d30142871ad Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 15:29:57 +0000 Subject: [PATCH 7/8] chore: Update RELEASE.md --- RELEASE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RELEASE.md b/RELEASE.md index ad597662..eec8c517 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -1,8 +1,8 @@ # Misti Release Checklist - [ ] Release new Misti version - - [ ] Run: `yarn release -- --dry-run` to ensure everything works as expected - - [ ] Run: `yarn release --` and follow the instructions + - [ ] Run: `yarn release --dry-run` to ensure everything works as expected + - [ ] Run: `yarn release` and follow the instructions - [ ] Create a GitHub release - [ ] Prepare [documentation](https://github.com/nowarp/nowarp.github.io/): - [ ] Update the supported Tact version in the introduction page From 080809c85500746439969c9173192d2007407d7d Mon Sep 17 00:00:00 2001 From: Georgiy Komarov Date: Thu, 31 Oct 2024 15:31:23 +0000 Subject: [PATCH 8/8] Release 0.5.0 --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 228a8e8f..da870355 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.0] - 2024-10-31 + ### Added - `SuspiciousMessageMode` detector: PR [#193](https://github.com/nowarp/misti/pull/193) - `SendInLoop` detector: PR [#168](https://github.com/nowarp/misti/pull/168) diff --git a/package.json b/package.json index 7fc6f05d..9a42d992 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nowarp/misti", - "version": "0.4.2", + "version": "0.5.0", "repository": { "type": "git", "url": "git+https://github.com/nowarp/misti.git"