Skip to content

Commit

Permalink
feat(lints): Add ZeroAddress lint
Browse files Browse the repository at this point in the history
  • Loading branch information
byakuren-hijiri committed May 29, 2024
1 parent acdab6e commit 66eee51
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 9 deletions.
45 changes: 45 additions & 0 deletions src/detectors/builtin/zeroAddress.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { Detector } from "../detector";
import { MistiContext } from "../../internals/context";
import { CompilationUnit } from "../../internals/ir";
import { foldExpressions } from "../../internals/tactASTUtil";
import { createError, MistiTactError, Severity } from "../../internals/errors";
import { ASTExpression } from "@tact-lang/compiler/dist/grammar/ast";

function findZeroAddress(
acc: MistiTactError[],
expr: ASTExpression,
): MistiTactError[] {
if (expr.kind === "op_static_call") {
if (
expr.name === "newAddress" &&
expr.args.length === 2 &&
expr.args[1].kind === "number" &&
expr.args[1].value === 0n
) {
acc.push(
createError("Using zero address", Severity.MEDIUM, expr.args[1].ref),
);
}
}
return acc;
}

/**
* A detector that identifies uses of zero address.
*
* Using the zero address in smart contracts is typically problematic because it can be
* exploited as a default or uninitialized address, leading to unintended transfers and
* security vulnerabilities. Additionally, operations involving the zero address can
* result in loss of funds or tokens, as there is no private key to access this address.
*/
export class ZeroAddress extends Detector {
get id(): string {
return "ZeroAddress";
}

check(_ctx: MistiContext, cu: CompilationUnit): MistiTactError[] {
return cu.ast.getProgramEntries().reduce((acc, node) => {
return acc.concat(foldExpressions(node, [], findZeroAddress));
}, [] as MistiTactError[]);
}
}
5 changes: 3 additions & 2 deletions src/detectors/detector.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { MistiContext } from "../internals/context";
import { CompilationUnit } from "../internals/ir";
import { MistiTactError, Severity } from "../internals/errors";
import { ASTRef } from "@tact-lang/compiler/dist/grammar/ast";
import { MistiTactError } from "../internals/errors";

/**
* Abstract base class for a detector module, providing an interface for defining various types of detectors.
Expand Down Expand Up @@ -31,6 +30,8 @@ const BuiltInDetectors: Record<string, () => Promise<Detector>> = {
import("./builtin/readOnlyVariables").then(
(module) => new module.ReadOnlyVariables(),
),
ZeroAddress: () =>
import("./builtin/zeroAddress").then((module) => new module.ZeroAddress()),
};

/**
Expand Down
1 change: 1 addition & 0 deletions src/internals/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const ConfigSchema = z.object({
/** Built-in detectors enabled by default, if no user configuration is provided. */
export const BUILTIN_DETECTORS: DetectorConfig[] = [
{ className: "ReadOnlyVariables" },
{ className: "ZeroAddress" },
];

/** Represents content of the Misti configuration file (misti.config.json). */
Expand Down
33 changes: 31 additions & 2 deletions src/internals/ir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
ASTReceive,
ASTField,
ASTInitFunction,
ASTNode,
ASTFunction,
ASTNativeFunction,
ASTConstant,
Expand All @@ -21,6 +22,7 @@ export type ProjectName = string;
export class TactASTStore {
/**
* Constructs a TactASTStore with mappings to all major AST components.
* @param programEntries Identifiers of AST elements defined on the top-level.
* @param functions Functions and methods including user-defined and special methods.
* @param constants Constants defined across the compilation unit.
* @param contracts Contracts defined within the project.
Expand All @@ -31,6 +33,7 @@ export class TactASTStore {
* @param statements All executable statements within all functions of the project.
*/
constructor(
private programEntries: Set<number>,
private functions: Map<number, ASTFunction | ASTReceive | ASTInitFunction>,
private constants: Map<number, ASTConstant>,
private contracts: Map<number, ASTContract>,
Expand All @@ -41,6 +44,32 @@ export class TactASTStore {
private statements: Map<number, ASTStatement>,
) {}

/**
* Returns program entries defined on the top-level.
*/
getProgramEntries(): ASTNode[] {
return Array.from(this.programEntries).reduce((acc, id) => {
if (this.functions.has(id)) {
acc.push(this.functions.get(id)!);
} else if (this.constants.has(id)) {
acc.push(this.constants.get(id)!);
} else if (this.contracts.has(id)) {
acc.push(this.contracts.get(id)!);
} else if (this.nativeFunctions.has(id)) {
acc.push(this.nativeFunctions.get(id)!);
} else if (this.primitives.has(id)) {
acc.push(this.primitives.get(id)!);
} else if (this.structs.has(id)) {
acc.push(this.structs.get(id)!);
} else if (this.traits.has(id)) {
acc.push(this.traits.get(id)!);
} else {
throw new Error(`No entry found for ID: ${id}`);
}
return acc;
}, [] as ASTNode[]);
}

/**
* Retrieves a function or method by its ID.
* @param id The unique identifier of the function or method.
Expand Down Expand Up @@ -190,7 +219,7 @@ export class TactASTStore {
* @param contractId The ID of the contract.
* @returns An array of constant IDs or undefined if no contract is found.
*/
public getConstants(contractId: number): number[] | undefined {
public getContractConstants(contractId: number): number[] | undefined {
const contract = this.getContract(contractId);
if (!contract) {
return undefined;
Expand All @@ -208,7 +237,7 @@ export class TactASTStore {
* @param contractId The ID of the contract.
* @returns An array of ASTField or undefined if no contract is found.
*/
public getFields(contractId: number): ASTField[] | undefined {
public getContractFields(contractId: number): ASTField[] | undefined {
const contract = this.getContract(contractId);
if (!contract) {
return undefined;
Expand Down
15 changes: 11 additions & 4 deletions src/internals/tactIRBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ function generateReceiveName(receive: ASTReceive): string {
* Transforms the TactAST imported from the tact compiler to a representation more suitable for analysis.
*/
export class ASTMapper {
private programEntries = new Set<number>();
private functions = new Map<
number,
ASTFunction | ASTReceive | ASTInitFunction
Expand All @@ -95,20 +96,26 @@ export class ASTMapper {

constructor(private ast: TactAST) {
this.ast.functions.forEach((func) => {
this.programEntries.add(func.id);
if (func.kind == "def_function") {
this.processFunction(func);
} else {
this.nativeFunctions.set(func.id, func);
}
});
this.ast.constants.forEach((constant) =>
this.constants.set(constant.id, constant),
);
this.ast.types.forEach((type) => this.processType(type));
this.ast.constants.forEach((constant) => {
this.programEntries.add(constant.id);
this.constants.set(constant.id, constant);
});
this.ast.types.forEach((type) => {
this.programEntries.add(type.id);
this.processType(type);
});
}

public getASTStore(): TactASTStore {
return new TactASTStore(
this.programEntries,
this.functions,
this.constants,
this.contracts,
Expand Down
6 changes: 5 additions & 1 deletion test/config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ jest.mock("fs");
describe("Config class", () => {
const MOCK_CONFIG_PATH = "./mistiConfig_mock.json";
const MOCK_CONFIG_CONTENT = JSON.stringify({
detectors: [{ className: "ReadOnlyVariables" }],
detectors: [
{ className: "ReadOnlyVariables" },
{ className: "ZeroAddress" },
],
ignored_projects: ["ignoredProject"],
});

Expand All @@ -19,6 +22,7 @@ describe("Config class", () => {
const configInstance = new MistiConfig(MOCK_CONFIG_PATH);
expect(configInstance.detectorsEnabled).toEqual([
{ className: "ReadOnlyVariables" },
{ className: "ZeroAddress" },
]);
expect(configInstance.ignoredProjects).toEqual(["ignoredProject"]);
});
Expand Down
25 changes: 25 additions & 0 deletions test/contracts/zero-address.cfg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"projectName": "zero-address",
"functions": [],
"contracts": [
{
"name": "SampleContract",
"methods": [
{
"name": "SampleContract.init",
"cfg": {
"nodes": [
{
"id": 606,
"stmtID": 8037,
"srcEdges": [],
"dstEdges": []
}
],
"edges": []
}
}
]
}
]
}
7 changes: 7 additions & 0 deletions test/contracts/zero-address.expected.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

/home/jubnzv/Dev/misti/test/contracts/zero-address.tact:3:23:
2 | init() {
> 3 | newAddress(1, 0x000000000000000000000000000000000000000000000000);
^
4 | }
Using zero address
5 changes: 5 additions & 0 deletions test/contracts/zero-address.tact
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
contract SampleContract {
init() {
newAddress(1, 0x000000000000000000000000000000000000000000000000);
}
}

0 comments on commit 66eee51

Please sign in to comment.