Add unchecked return scanner plugin

packages/scanner-core/src/plugin-registry.test.ts

@@ -46,6 +46,16 @@ describe('PluginRegistry', () => {
     expect(registry.size).toBe(1);
   });
 
+  it('should register initial plugins from the constructor', () => {
+    const registryWithInitialPlugins = new PluginRegistry([
+      createMockPlugin('reentrancy'),
+      createMockPlugin('unchecked-return'),
+    ]);
+
+    expect(registryWithInitialPlugins.size).toBe(2);
+    expect(registryWithInitialPlugins.get('unchecked-return')?.metadata.category).toBe('CUSTOM');
+  });
+
   it('should retrieve a registered plugin', () => {
     const plugin = createMockPlugin('test-plugin');
     registry.register(plugin);

packages/scanner-core/src/plugin-registry.ts

@@ -4,6 +4,10 @@ import type { AnalysisContext, IRulePlugin, PluginMetadata } from '@veridion/sca
 export class PluginRegistry {
   private plugins = new Map<string, IRulePlugin>();
 
+  constructor(initialPlugins: IRulePlugin[] = []) {
+    this.registerAll(initialPlugins);
+  }
+
   register(plugin: IRulePlugin): void {
     if (this.plugins.has(plugin.metadata.id)) {
       logger.warn({ pluginId: plugin.metadata.id }, 'Plugin already registered, overwriting');

plugins/unchecked-return/.eslintrc.js

@@ -0,0 +1,4 @@
+const { createConfig } = require('@veridion/eslint-config/base');
+
+/** @type {import('eslint').Linter.Config} */
+module.exports = createConfig(__dirname);

plugins/unchecked-return/package.json

@@ -0,0 +1,28 @@
+{
+  "name": "@veridion/plugin-unchecked-return",
+  "version": "0.1.0",
+  "private": true,
+  "description": "Unchecked low-level call return detection plugin for Veridion scanner",
+  "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",
+    "clean": "rm -rf dist"
+  },
+  "dependencies": {
+    "@veridion/scanner-types": "workspace:*",
+    "@veridion/shared": "workspace:*",
+    "@veridion/logger": "workspace:*"
+  },
+  "devDependencies": {
+    "@veridion/eslint-config": "workspace:*",
+    "@veridion/tsconfig": "workspace:*",
+    "eslint": "^8.57.0",
+    "typescript": "^5.4.5",
+    "vitest": "^1.6.0"
+  }
+}

plugins/unchecked-return/src/index.test.ts

@@ -0,0 +1,173 @@
+import type { AnalysisContext } from '@veridion/scanner-types';
+import { FindingSeverity } from '@veridion/shared';
+import { describe, expect, it } from 'vitest';
+
+import { UncheckedReturnPlugin } from './index';
+
+describe('UncheckedReturnPlugin', () => {
+  const plugin = new UncheckedReturnPlugin();
+
+  function createContext(sourceCode: string): AnalysisContext {
+    return {
+      contractName: 'Vault',
+      sourceCode,
+      chain: 'ethereum',
+      language: 'solidity',
+      compilerVersion: '0.8.19',
+      metadata: {},
+    };
+  }
+
+  it('should have correct metadata', () => {
+    expect(plugin.metadata.id).toBe('unchecked-return');
+    expect(plugin.metadata.severity).toBe('HIGH');
+    expect(plugin.metadata.category).toBe('UNCHECKED_RETURN');
+    expect(plugin.metadata.languages).toContain('solidity');
+  });
+
+  it('should support solidity on EVM chains', () => {
+    expect(plugin.supportsContext(createContext(''))).toBe(true);
+  });
+
+  it('should reject unsupported languages', () => {
+    expect(
+      plugin.supportsContext({
+        ...createContext(''),
+        language: 'rust',
+      }),
+    ).toBe(false);
+  });
+
+  it('should detect unchecked call return values', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    function withdraw(address target, bytes memory data) external {
+        target.call(data);
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(1);
+    expect(findings[0]?.title).toContain('call');
+    expect(findings[0]?.recommendation).toContain('Capture the boolean return value');
+  });
+
+  it('should detect assigned but unchecked call return values', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    function withdraw(address target, bytes memory data) external {
+        (bool success, bytes memory result) = target.call(data);
+        result;
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(1);
+    expect(findings[0]?.recommendation).toContain('success');
+  });
+
+  it('should detect unchecked send return values', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    function refund(address payable target, uint256 amount) external {
+        target.send(amount);
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(1);
+    expect(findings[0]?.title).toContain('send');
+  });
+
+  it('should detect unchecked delegatecall return values', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Proxy {
+    function forward(address implementation, bytes memory data) external {
+        (bool ok, ) = implementation.delegatecall(data);
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(1);
+    expect(findings[0]?.title).toContain('delegatecall');
+  });
+
+  it('should not flag call return values checked with require', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    function withdraw(address target, bytes memory data) external {
+        (bool success, ) = target.call(data);
+        require(success, "low-level call failed");
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(0);
+  });
+
+  it('should not flag send return values checked inline', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    function refund(address payable target, uint256 amount) external {
+        if (!target.send(amount)) {
+            revert("refund failed");
+        }
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(0);
+  });
+
+  it('should not flag delegatecall values checked with a branch', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Proxy {
+    function forward(address implementation, bytes memory data) external {
+        (bool ok, ) = implementation.delegatecall(data);
+        if (!ok) {
+            revert("delegatecall failed");
+        }
+    }
+}`),
+    );
+
+    expect(findings).toHaveLength(0);
+  });
+
+  it('should ignore comments that mention low-level calls', async () => {
+    const findings = await plugin.analyze(
+      createContext(`
+contract Vault {
+    // target.call(data);
+    function ok() external {}
+}`),
+    );
+
+    expect(findings).toHaveLength(0);
+  });
+
+  it('should provide a fix recommendation for findings', () => {
+    const recommendation = plugin.getFixRecommendation({
+      pluginId: 'unchecked-return',
+      title: 'Unchecked call Return Value',
+      description: '',
+      severity: FindingSeverity.HIGH,
+      filePath: 'Vault.sol',
+      lineStart: 3,
+      lineEnd: 3,
+      codeSnippet: 'target.call(data);',
+      recommendation: '',
+      confidence: 0.9,
+      references: [],
+    });
+
+    expect(recommendation).toContain('require(success');
+  });
+});

plugins/unchecked-return/src/index.ts

@@ -0,0 +1,162 @@
+import type {
+  AnalysisContext,
+  FindingResult,
+  IRulePlugin,
+  PluginMetadata,
+} from '@veridion/scanner-types';
+import { FindingSeverity } from '@veridion/shared';
+
+const LOW_LEVEL_CALL_PATTERN = /\.(call|send|delegatecall)\s*(?:\{[^}]*\})?\s*\(/g;
+const EVM_CHAINS = ['ethereum', 'polygon', 'bsc', 'avalanche', 'arbitrum', 'optimism'];
+const CHECK_LOOKAHEAD_LINES = 12;
+
+const metadata: PluginMetadata = {
+  id: 'unchecked-return',
+  name: 'Unchecked Return Value Detector',
+  version: '1.0.0',
+  description:
+    'Detects low-level Solidity call, send, and delegatecall results that are ignored or assigned without validation.',
+  severity: FindingSeverity.HIGH,
+  category: 'UNCHECKED_RETURN',
+  chains: EVM_CHAINS,
+  languages: ['solidity'],
+  tags: ['unchecked-return', 'low-level-call', 'call', 'send', 'delegatecall'],
+  author: 'Veridion',
+  references: [
+    'https://swcregistry.io/docs/SWC-104',
+    'https://consensys.github.io/smart-contract-best-practices/development-recommendations/general/external-calls/',
+  ],
+};
+
+export class UncheckedReturnPlugin implements IRulePlugin {
+  readonly metadata = metadata;
+
+  async initialize(_config?: Record<string, unknown>): Promise<void> {
+    // noop
+  }
+
+  // eslint-disable-next-line @typescript-eslint/require-await
+  async analyze(context: AnalysisContext): Promise<FindingResult[]> {
+    const findings: FindingResult[] = [];
+    const lines = context.sourceCode.split('\n');
+
+    for (let i = 0; i < lines.length; i++) {
+      const sourceLine = lines[i];
+      if (!sourceLine) continue;
+
+      const codeLine = stripInlineComment(sourceLine);
+      LOW_LEVEL_CALL_PATTERN.lastIndex = 0;
+
+      let match: RegExpExecArray | null;
+      while ((match = LOW_LEVEL_CALL_PATTERN.exec(codeLine)) !== null) {
+        const callKind = match[1];
+        if (!callKind || isInlineChecked(codeLine, match.index)) continue;
+
+        const checkedVariable = extractCheckedVariable(codeLine, match.index, callKind);
+        if (checkedVariable && isVariableChecked(lines, i, checkedVariable)) continue;
+
+        findings.push(createFinding(context, sourceLine, i + 1, callKind, checkedVariable));
+      }
+    }
+
+    return findings;
+  }
+
+  getFixRecommendation(finding: FindingResult): string {
+    return `Handle the boolean return from the low-level call at ${finding.filePath}:${finding.lineStart}. Assign it to a variable and enforce it with require(success, "low-level call failed") or revert inside an if (!success) branch.`;
+  }
+
+  supportsContext(context: AnalysisContext): boolean {
+    return (
+      this.metadata.chains.includes(context.chain) &&
+      this.metadata.languages.includes(context.language)
+    );
+  }
+}
+
+function stripInlineComment(line: string): string {
+  const commentIndex = line.indexOf('//');
+  return commentIndex === -1 ? line : line.slice(0, commentIndex);
+}
+
+function isInlineChecked(line: string, matchIndex: number): boolean {
+  const prefix = line.slice(0, matchIndex);
+
+  return (
+    /\b(?:require|assert)\s*\([^;]*$/.test(prefix) ||
+    /\bif\s*\([^;]*$/.test(prefix) ||
+    /\breturn\s+[^;]*$/.test(prefix)
+  );
+}
+
+function extractCheckedVariable(line: string, matchIndex: number, callKind: string): string | null {
+  const prefix = line.slice(0, matchIndex);
+
+  if (callKind === 'send') {
+    const boolAssignment = prefix.match(/\b(?:bool\s+)?([A-Za-z_$][\w$]*)\s*=\s*[^=]*$/);
+    return boolAssignment?.[1] ?? null;
+  }
+
+  const tupleAssignment = prefix.match(/\(\s*(?:bool\s+)?([A-Za-z_$][\w$]*)\s*,/);
+  return tupleAssignment?.[1] ?? null;
+}
+
+function isVariableChecked(lines: string[], startIndex: number, variableName: string): boolean {
+  if (variableName === '_') return false;
+
+  const escapedName = escapeRegExp(variableName);
+  const variablePattern = new RegExp(`\\b${escapedName}\\b`);
+  const positiveGuardPattern = new RegExp(
+    `\\b(?:require|assert)\\s*\\(\\s*${escapedName}(?:\\s*(?:==|!=)\\s*(?:true|false))?\\b`,
+  );
+  const negativeGuardPattern = new RegExp(`\\bif\\s*\\(\\s*!\\s*${escapedName}\\b`);
+  const branchGuardPattern = new RegExp(`\\bif\\s*\\(\\s*${escapedName}\\b`);
+
+  for (let i = startIndex; i < Math.min(startIndex + CHECK_LOOKAHEAD_LINES, lines.length); i++) {
+    const currentLine = stripInlineComment(lines[i] ?? '');
+    if (!variablePattern.test(currentLine)) continue;
+
+    if (
+      positiveGuardPattern.test(currentLine) ||
+      negativeGuardPattern.test(currentLine) ||
+      branchGuardPattern.test(currentLine)
+    ) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+function createFinding(
+  context: AnalysisContext,
+  sourceLine: string,
+  lineNumber: number,
+  callKind: string,
+  checkedVariable: string | null,
+): FindingResult {
+  const recommendation =
+    checkedVariable === null
+      ? 'Capture the boolean return value and require it before continuing, for example: (bool success, ) = target.call(data); require(success, "low-level call failed");'
+      : `Validate ${checkedVariable} with require(${checkedVariable}, "low-level call failed") or revert when it is false.`;
+
+  return {
+    pluginId: metadata.id,
+    title: `Unchecked ${callKind} Return Value`,
+    description:
+      `The result of a low-level Solidity ${callKind} operation is not checked. ` +
+      'Ignoring this return value can let execution continue after a failed external call, leaving contract state inconsistent.',
+    severity: metadata.severity,
+    filePath: `${context.contractName}.sol`,
+    lineStart: lineNumber,
+    lineEnd: lineNumber,
+    codeSnippet: sourceLine.trim(),
+    recommendation,
+    confidence: checkedVariable === null ? 0.9 : 0.85,
+    references: metadata.references ?? [],
+  };
+}
+
+function escapeRegExp(value: string): string {
+  return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+}