Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions plugins/unchecked-return/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const { createConfig } = require('@veridion/eslint-config/base');

/** @type {import('eslint').Linter.Config} */
module.exports = createConfig(__dirname);
30 changes: 30 additions & 0 deletions plugins/unchecked-return/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"name": "@veridion/plugin-unchecked-return",
"version": "0.1.0",
"private": true,
"description": "Unchecked low-level call return value detection",
"main": "./src/index.ts",
"types": "./src/index.ts",
"scripts": {
"build": "tsc",
"typecheck": "tsc --noEmit",
"lint": "eslint src/ --max-warnings 0",
"test": "vitest run",
"test:watch": "vitest",
"test:coverage": "vitest run --coverage",
"clean": "rm -rf dist"
},
"dependencies": {
"@veridion/scanner-types": "workspace:*",
"@veridion/shared": "workspace:*"
},
"devDependencies": {
"@veridion/eslint-config": "workspace:*",
"@veridion/scanner-core": "workspace:*",
"@veridion/tsconfig": "workspace:*",
"@vitest/coverage-v8": "^1.6.0",
"eslint": "^8.57.0",
"typescript": "^5.4.5",
"vitest": "^1.6.0"
}
}
142 changes: 142 additions & 0 deletions plugins/unchecked-return/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { PluginRegistry } from '@veridion/scanner-core';
import type { AnalysisContext } from '@veridion/scanner-types';
import { describe, expect, it } from 'vitest';

import { UncheckedReturnPlugin } from './index';

const plugin = new UncheckedReturnPlugin();
const context = (body: string): AnalysisContext => ({
contractName: 'Example',
sourceCode: `contract Example { function run(address payable target) external { ${body} } }`,
chain: 'ethereum',
language: 'solidity',
compilerVersion: '0.8.20',
metadata: {},
});

// These snippets exercise source analysis only; they are never deployed or executed.
describe('UncheckedReturnPlugin', () => {
it.each([
'target.call("");',
'target.send(1);',
'target.delegatecall(data);',
'target.staticcall(data);',
'target.call{value: 1, gas: 5000}(abi.encode(a, b));',
'target.call.value(1).gas(5000)(data);',
'target.call.gas(5000).value(1)(data);',
'address(target).call(data);',
'targets[0].send(1);',
'wallet.target.send(1);',
'(bool ok, ) = target.call(data);',
'(, bytes memory data) = target.call("");',
'bool ok = target.send(1);',
'(bool ok, ) = target.call(data); require(other);',
'(bool ok, ) = target.call(data); require(ok == trueFlag);',
'(bool ok, ) = target.call(data); require(untrue == ok);',
'(bool ok, ) = target.call(data); require(ok || allowed);',
'(bool ok, ) = target.call(data); require(!ok);',
'(bool ok, ) = target.call(data); if (allowed) { require(ok); }',
'(bool ok, ) = target.call(data); ok = true; require(ok);',
'(bool ok, ) = target.call(data); } function other() public { require(ok);',
'(bool ok, ) = target.call(data); if (!ok) { emit Failed(); }',
'bool ok = !target.send(1); require(ok);',
'require(wrapper(target.send(1)));',
'emit Sent(target.send(1));',
'if (allowed) target.send(1);',
'require(target.send(1) || allowed);',
'require(target.send(1) == trueFlag);',
'target /* comment */ . call ( data );',
'((boolOk, bytesData) = target.call(data));',
])('reports an unrecognized or missing check: %s', async (body) => {
const findings = await plugin.analyze(context(body));
expect(findings).toHaveLength(1);
expect(findings[0]?.recommendation).toContain('require(success');
});

it.each([
'',
'target.transfer(1);',
'// target.call(data);\n',
'/* target.send(1); */',
'string memory note = "target.call(data);";',
"string memory note = 'target.send(1);';",
String.raw`string memory note = "escaped \" target.call(data);";`,
'require(target.send(1));',
'assert(target.send(1));',
'require(payable(target).send(1), "failed");',
'require(target.send(1) == true);',
'if (!target.send(1)) revert Failed();',
'if (target.send(1) == false) { revert(); }',
'(bool ok, ) = target.call(data); require(ok);',
'(bool ok, bytes memory data) = target.delegatecall(data); require(ok, "failed");',
'(ok, ) = target.staticcall(data); assert(ok);',
'bool ok = target.send(1); require(ok);',
'ok = target.send(1); require(ok);',
'(bool ok, ) = target.call{value: 1}(data); require(ok);',
'(bool ok, ) = target.call(data); require(((ok)));',
'(bool ok, ) = target.call(data); require(ok == true);',
'(bool ok, ) = target.call(data); require(true == ok);',
'(bool ok, ) = target.call(data); require(ok != false);',
'(bool ok, ) = target.call(data); require(false != ok);',
'(bool ok, ) = target.call(data); if (!ok) revert Failed();',
'(bool ok, ) = target.call(data); if (ok == false) { revert("failed"); }',
'(bool ok, ) = target.call(data); return ok;',
'return target.call(data);',
'return target.send(1);',
'target.call;',
'call(data);',
])('leaves checked, forwarded, or unrelated code alone: %s', async (body) => {
expect(await plugin.analyze(context(body))).toEqual([]);
});

it('preserves source locations across comments and multiline call options', async () => {
const input = context('');
input.sourceCode =
'/* intro\n comment */\ncontract Example {\nfunction run() external {\n target.call{\n value: 1\n }(data);\n}\n}';
const [finding] = await plugin.analyze(input);
expect(finding).toMatchObject({
pluginId: 'unchecked-return',
severity: 'MEDIUM',
filePath: 'Example.sol',
lineStart: 5,
lineEnd: 7,
codeSnippet: 'target.call{\n value: 1\n }(data)',
});
expect(finding && plugin.getFixRecommendation(finding)).toContain('require(success');
});

it('does not share checks between calls or between analyses', async () => {
const input = context('target.call(data); (bool ok, ) = target.call(data); require(ok);');
expect(await plugin.analyze(input)).toHaveLength(1);
expect(await plugin.analyze(input)).toHaveLength(1);
expect(await plugin.analyze(context('target.send(1); target.call(data);'))).toHaveLength(2);
});

it.each(['target.call(', 'target.call{value: 1', 'target.call.value(', '/* unfinished'])(
'handles incomplete source: %s',
async (body) => {
await expect(plugin.analyze({ ...context(''), sourceCode: body })).resolves.toEqual([]);
},
);

it('supports only declared chains and Solidity', async () => {
await expect(plugin.initialize()).resolves.toBeUndefined();
expect(plugin.supportsContext(context(''))).toBe(true);
for (const input of [
{ ...context('target.send(1);'), chain: 'stellar' },
{ ...context('target.send(1);'), language: 'rust' },
]) {
expect(plugin.supportsContext(input)).toBe(false);
expect(await plugin.analyze(input)).toEqual([]);
}
});

it('registers and runs through the existing registry', async () => {
const registry = new PluginRegistry();
registry.register(plugin);
expect(registry.getByCategory('UNCHECKED_RETURN')).toEqual([plugin]);
const [registered] = registry.getMatchingPlugins(context(''));
expect(registered).toBe(plugin);
expect(await registered?.analyze(context('target.send(1);'))).toHaveLength(1);
});
});
206 changes: 206 additions & 0 deletions plugins/unchecked-return/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
import type {
AnalysisContext,
FindingResult,
IRulePlugin,
PluginMetadata,
} from '@veridion/scanner-types';
import { FindingSeverity } from '@veridion/shared';

interface Token {
value: string;
start: number;
end: number;
}

const methods = new Set(['call', 'send', 'delegatecall', 'staticcall']);
const references = ['https://swcregistry.io/docs/SWC-104/'];

function tokenize(source: string): Token[] {
const pattern =
/\/\/[^\r\n]*|\/\*[\s\S]*?(?:\*\/|$)|"(?:\\[\s\S]|[^"\\])*"|'(?:\\[\s\S]|[^'\\])*'|[a-zA-Z_$][\w$]*|\d+|==|!=|&&|\|\||=>|[^\s]/g;
return Array.from(source.matchAll(pattern))
.filter(([value]) => !value.startsWith('//') && !value.startsWith('/*'))
.map((match) => ({ value: match[0], start: match.index, end: match.index + match[0].length }));
}

function pairs(tokens: Token[]): Map<number, number> {
const result = new Map<number, number>();
const stack: number[] = [];
const closing: Record<string, string> = { ')': '(', ']': '[', '}': '{' };
tokens.forEach(({ value }, index) => {
if (['(', '[', '{'].includes(value)) stack.push(index);
else if (closing[value]) {
const start = stack.pop();
if (start !== undefined && tokens[start]?.value === closing[value]) {
result.set(start, index);
result.set(index, start);
}
}
});
return result;
}

function text(tokens: Token[]): string {
return tokens.map((token) => token.value).join(' ');
}

function unwrap(tokens: Token[]): Token[] {
while (tokens[0]?.value === '(' && pairs(tokens).get(0) === tokens.length - 1) {
tokens = tokens.slice(1, -1);
}
return tokens;
}

function checks(tokens: Token[], name: string, success: boolean): boolean {
tokens = unwrap(tokens);
const value = text(tokens);
const literal = success ? 'true' : 'false';
const opposite = success ? 'false' : 'true';
if (value === name) return success;
if (tokens[0]?.value === '!') return checks(tokens.slice(1), name, !success);
return (
value === `${name} == ${literal}` ||
value === `${literal} == ${name}` ||
value === `${name} != ${opposite}` ||
value === `${opposite} != ${name}`
);
}

function guarded(tokens: Token[], name: string): boolean {
const keyword = tokens[0]?.value;
if (keyword === 'return') return text(tokens.slice(1, 3)) === `${name} ;`;
if (!['require', 'assert', 'if'].includes(keyword ?? '') || tokens[1]?.value !== '(') {
return false;
}
const end = pairs(tokens).get(1);
if (end === undefined) return false;
let condition = tokens.slice(2, end);
const comma = condition.findIndex((token) => token.value === ',');
if (comma !== -1 && keyword !== 'if') condition = condition.slice(0, comma);
if (keyword !== 'if') return checks(condition, name, true);
if (!checks(condition, name, false)) return false;
const body = tokens.slice(end + 1);
return body[0]?.value === 'revert' || (body[0]?.value === '{' && body[1]?.value === 'revert');
}

function statementStart(tokens: Token[], index: number, matched: Map<number, number>): number {
for (let i = index - 1; i >= 0; i--) {
const value = tokens[i]?.value;
if (value === ')' || value === ']') {
i = matched.get(i) ?? i;
} else if (value === ';' || value === '{' || value === '}') {
return i + 1;
}
}
return 0;
}

function receiverStart(tokens: Token[], index: number, matched: Map<number, number>): number {
let start = index;
while (start > 0) {
if (tokens[start]?.value === ')' || tokens[start]?.value === ']') {
const open = matched.get(start);
if (open === undefined) break;
start = open;
if (/^[a-zA-Z_$]/.test(tokens[start - 1]?.value ?? '')) start--;
} else if (tokens[start - 1]?.value === '.') {
start -= 2;
} else break;
}
return start;
}

function capturedName(tokens: Token[], method: string): string | undefined {
const lhs = text(tokens);
const match =
method === 'send'
? /^(?:bool )?([a-zA-Z_$][\w$]*) =$/.exec(lhs)
: /^\( (?:bool )?([a-zA-Z_$][\w$]*) ,[^]* \) =$/.exec(lhs);
return match?.[1];
}

const metadata: PluginMetadata = {
id: 'unchecked-return',
name: 'Unchecked Return Value Detector',
version: '1.0.0',
description: 'Detects low-level Solidity calls without a recognized success check.',
severity: FindingSeverity.MEDIUM,
category: 'UNCHECKED_RETURN',
chains: ['ethereum', 'polygon', 'bsc', 'avalanche', 'arbitrum', 'optimism'],
languages: ['solidity'],
tags: ['unchecked-return', 'low-level-call', 'swc-104'],
references,
};

export class UncheckedReturnPlugin implements IRulePlugin {
readonly metadata = metadata;

initialize(): Promise<void> {
return Promise.resolve();
}

analyze(context: AnalysisContext): Promise<FindingResult[]> {
if (!this.supportsContext(context)) return Promise.resolve([]);
const tokens = tokenize(context.sourceCode);
const matched = pairs(tokens);
const findings: FindingResult[] = [];

tokens.forEach((token, index) => {
if (!methods.has(token.value) || tokens[index - 1]?.value !== '.') return;
let open = index + 1;
if (tokens[open]?.value === '{') open = (matched.get(open) ?? tokens.length) + 1;
// Solidity before 0.7 used call.value(...).gas(...)(...).
while (
tokens[open]?.value === '.' &&
['value', 'gas'].includes(tokens[open + 1]?.value ?? '')
) {
open = (matched.get(open + 2) ?? tokens.length) + 1;
}
if (tokens[open]?.value !== '(') return;
const end = matched.get(open);
if (end === undefined) return;
const start = statementStart(tokens, index, matched);
const receiver = receiverStart(tokens, index - 2, matched);
const prefix = tokens.slice(start, receiver);
const name = capturedName(prefix, token.value);
const after = tokens.slice(end + 1);
if (name && after[0]?.value === ';' && guarded(after.slice(1), name)) return;
if (prefix[0]?.value === 'return' && after[0]?.value === ';') return;

// Only send() returns a single bool that can be checked inline.
if (token.value === 'send') {
const callStart = prefix.findIndex(({ value }) => value === '(');
const placeholder: Token = { value: 'result', start: token.start, end: token.end };
if (callStart !== -1 && guarded([...prefix, placeholder, ...after], 'result')) return;
}

const first = tokens[start] ?? token;
const last = tokens[end] ?? token;
findings.push({
pluginId: metadata.id,
title: `Unchecked ${token.value}() return value`,
description: `No recognized success check follows this ${token.value}() call. A failed low-level call returns false instead of reverting the caller.`,
severity: metadata.severity,
filePath: `${context.contractName}.sol`,
lineStart: context.sourceCode.slice(0, first.start).split('\n').length,
lineEnd: context.sourceCode.slice(0, last.end).split('\n').length,
codeSnippet: context.sourceCode.slice(first.start, last.end),
recommendation:
token.value === 'send'
? 'Capture the boolean result: bool success = recipient.send(amount); require(success, "Send failed");'
: `Capture the success flag: (bool success, ) = target.${token.value}(data); require(success, "Call failed"); Preserve any original value and gas options.`,
confidence: 0.7,
references: [...references],
});
});
return Promise.resolve(findings);
}

getFixRecommendation(finding: FindingResult): string {
return finding.recommendation;
}

supportsContext(context: AnalysisContext): boolean {
return metadata.languages.includes(context.language) && metadata.chains.includes(context.chain);
}
}
9 changes: 9 additions & 0 deletions plugins/unchecked-return/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "@veridion/tsconfig/base.json",
"compilerOptions": {
"outDir": "./dist",
"rootDir": "./src"
},
"include": ["src/**/*.ts"],
"exclude": ["node_modules", "dist"]
}
Loading