Skip to content

Commit

Permalink
fix(exitCodeUsage): Report direct cases
Browse files Browse the repository at this point in the history
Closes #218
  • Loading branch information
jubnzv committed Nov 19, 2024
1 parent 9731fc6 commit e1fe023
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed
- Missing Module `version-info` When Installing Misti from GitHub: Issue [#216](https://github.com/nowarp/misti/issues/216/)
- `ExitCodeUsage` Handle direct cases: Issue [#218](https://github.com/nowarp/misti/issues/218/)

## [0.5.0] - 2024-10-31

Expand Down
108 changes: 80 additions & 28 deletions src/detectors/builtin/exitCodeUsage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,34 @@ import {
} from "../../internals/lattice";
import { Interval, Num } from "../../internals/numbers";
import { WideningWorklistSolver } from "../../internals/solver";
import { evalToType } from "../../internals/tact";
import { findInExpressions } from "../../internals/tact/iterators";
import { Transfer } from "../../internals/transfer";
import { MistiTactWarning, Severity } from "../../internals/warnings";
import { DataflowDetector } from "../detector";
import {
AstStatement,
AstId,
idText,
AstExpression,
AstStatementAssign,
AstStatementLet,
AstNumber,
} from "@tact-lang/compiler/dist/grammar/ast";

/**
* The minimum allowed value for user-defined exit codes.
* @remarks Values below 256 are reserved:
* - 0-127: Reserved for TVM/FunC
* - 128-255: Reserved for Tact
*/
const LOWER_BOUND = 256n;

/**
* The maximum allowed value for user-defined exit codes.
* @remarks Values above 65535 are invalid in TON smart contracts
*/
const UPPER_BOUND = 65535n;

type Variable = string & { readonly __brand: unique symbol };

type VariableState = Map<Variable, Interval>;
Expand Down Expand Up @@ -235,27 +249,66 @@ export class ExitCodeUsage extends DataflowDetector {
if (!stmt) {
throw InternalException.make(`Cannot find a statement for BB #${bb.idx}`);
}
// TODO: Handle direct cases e.g. throw(128)
const exitVariable = this.findExitVariable(stmt);
if (exitVariable === null) {
return;
const arg = this.getThrowFunctionArg(stmt);
if (!arg) return;
this.checkDirectExitCode(arg, warnings);
this.checkVariableExitCode(arg, state, warnings);
}

/**
* Checks for invalid exit codes specified directly as numbers (e.g., throw(128))
* @param arg The argument passed to the throw function
* @param warnings Array to collect any warnings found
*/
private checkDirectExitCode(
arg: AstExpression,
warnings: MistiTactWarning[],
): void {
const value = evalToType(arg, "bigint");
if (
value !== undefined &&
value !== null &&
typeof value === "bigint" &&
(value < LOWER_BOUND || value > UPPER_BOUND)
) {
warnings.push(
this.makeWarning(`Value is outside allowed range`, arg.loc, {
extraDescription: `Exit codes 0-255 are reserved. Used value: ${value}`,
suggestion: `Use a value between ${Number(LOWER_BOUND)} and ${Number(UPPER_BOUND)}`,
}),
);
}
const exitVariableName = idText(exitVariable);
for (const [varName, interval] of state.entries()) {
if (
exitVariableName === varName &&
this.isOutsideAllowedRange(interval)
) {
warnings.push(
this.makeWarning(
`Exit code variable "${varName}" has value outside allowed range`,
exitVariable.loc,
{
extraDescription: `Exit codes 0-255 are reserved. Variable value: ${interval.toString()}`,
suggestion: "Use a value between 256 and 65535",
},
),
);
}

/**
* Checks for invalid exit codes stored in variables
* @param arg The argument passed to the throw function
* @param state Current state containing variable intervals
* @param warnings Array to collect any warnings found
*/
private checkVariableExitCode(
arg: AstExpression,
state: VariableState,
warnings: MistiTactWarning[],
): void {
if (arg.kind === "id") {
const exitVariableName = idText(arg);
for (const [varName, interval] of state.entries()) {
if (
exitVariableName === varName &&
this.isOutsideAllowedRange(interval)
) {
warnings.push(
this.makeWarning(
`Exit code variable "${varName}" has value outside allowed range`,
arg.loc,
{
extraDescription: `Exit codes 0-255 are reserved. Variable value: ${interval.toString()}`,
suggestion: `Use a value between ${Number(LOWER_BOUND)} and ${Number(UPPER_BOUND)}`,
},
),
);
}
}
}
}
Expand All @@ -265,17 +318,17 @@ export class ExitCodeUsage extends DataflowDetector {
const upperBound = interval.high;

// Developer-allowed range is 256 to 65535
const belowMin = Num.compare(upperBound, Num.int(256n)) < 0;
const aboveMax = Num.compare(lowerBound, Num.int(65535n)) > 0;
const belowMin = Num.compare(upperBound, Num.int(LOWER_BOUND)) < 0;
const aboveMax = Num.compare(lowerBound, Num.int(UPPER_BOUND)) > 0;

return belowMin || aboveMax;
}

/**
* Finds a local variable used as an exit code.
* Returns first argument of throw functions or null if it's not a throw call
*/
private findExitVariable(stmt: AstStatement): AstId | null {
let result: AstId | null = null;
private getThrowFunctionArg(stmt: AstStatement): AstExpression | null {
let result: AstExpression | null = null;
// The first argument of these functions is an exit code:
// https://docs.tact-lang.org/ref/core-debug/#throw
const throwFunctions = new Set([
Expand All @@ -288,8 +341,7 @@ export class ExitCodeUsage extends DataflowDetector {
if (
expr.kind === "static_call" &&
expr.args.length > 0 &&
throwFunctions.has(idText(expr.function)) &&
expr.args[0].kind === "id"
throwFunctions.has(idText(expr.function))
) {
result = expr.args[0];
return true;
Expand Down
18 changes: 16 additions & 2 deletions src/internals/tact/constEval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ import {
} from "@tact-lang/compiler/dist/types/types";
import { Address, Cell, Slice } from "@ton/core";

/**
* A type that can be used to check the type of a constant expression.
*/
export type ExpectedType =
| "bigint"
| "boolean"
| "string"
| "null"
| "Address"
| "Cell"
| "Slice"
| "CommentValue"
| "StructValue";

/**
* Evaluates a constant expression and returns its value.
*
Expand All @@ -32,7 +46,7 @@ export function evalExpr(expr: AstExpression): Value | undefined {
*/
export function evalToType(
expr: AstExpression,
expectedType: string,
expectedType: ExpectedType,
): Value | undefined {
const value = evalExpr(expr);
return value !== undefined && checkType(value, expectedType)
Expand All @@ -52,7 +66,7 @@ export function evalToType(
*/
export function evalsToValue(
expr: AstExpression,
expectedType: string,
expectedType: ExpectedType,
expectedValue: Value,
): boolean {
const value = evalToType(expr, expectedType);
Expand Down
36 changes: 28 additions & 8 deletions test/detectors/ExitCodeUsage.expected.out
Original file line number Diff line number Diff line change
@@ -1,19 +1,39 @@
[HIGH] ExitCodeUsage: Value is outside allowed range
test/detectors/ExitCodeUsage.tact:14:28:
13 | // Bad
> 14 | nativeThrowUnless(128, sender() == self.owner);
^
15 |
Exit codes 0-255 are reserved. Used value: 128
Help: Use a value between 256 and 65535
See: https://nowarp.io/tools/misti/docs/detectors/ExitCodeUsage

[HIGH] ExitCodeUsage: Value is outside allowed range
test/detectors/ExitCodeUsage.tact:17:28:
16 | // Bad
> 17 | nativeThrowUnless(255, sender() == self.owner);
^
18 |
Exit codes 0-255 are reserved. Used value: 255
Help: Use a value between 256 and 65535
See: https://nowarp.io/tools/misti/docs/detectors/ExitCodeUsage

[HIGH] ExitCodeUsage: Exit code variable "code1" has value outside allowed range
test/detectors/ExitCodeUsage.tact:15:28:
14 | let code1: Int = 128;
> 15 | nativeThrowUnless(code1, sender() == self.owner);
test/detectors/ExitCodeUsage.tact:21:28:
20 | let code1: Int = 128;
> 21 | nativeThrowUnless(code1, sender() == self.owner);
^
16 |
22 |
Exit codes 0-255 are reserved. Variable value: 128
Help: Use a value between 256 and 65535
See: https://nowarp.io/tools/misti/docs/detectors/ExitCodeUsage

[HIGH] ExitCodeUsage: Exit code variable "code2" has value outside allowed range
test/detectors/ExitCodeUsage.tact:20:28:
19 | code2 = code2 - 10;
> 20 | nativeThrowUnless(code2, sender() == self.owner);
test/detectors/ExitCodeUsage.tact:26:28:
25 | code2 = code2 - 10;
> 26 | nativeThrowUnless(code2, sender() == self.owner);
^
21 |
27 |
Exit codes 0-255 are reserved. Variable value: 246
Help: Use a value between 256 and 65535
See: https://nowarp.io/tools/misti/docs/detectors/ExitCodeUsage
6 changes: 6 additions & 0 deletions test/detectors/ExitCodeUsage.tact
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
}

receive("test") {
// Bad
nativeThrowUnless(128, sender() == self.owner);

// Bad
nativeThrowUnless(255, sender() == self.owner);

// Bad
let code1: Int = 128;
nativeThrowUnless(code1, sender() == self.owner);
Expand Down

0 comments on commit e1fe023

Please sign in to comment.