From f68b69c1e7b0e9fd433b897e8fbf4c3b2717744d Mon Sep 17 00:00:00 2001 From: kitWarse Date: Mon, 29 Jun 2026 09:30:27 +0100 Subject: [PATCH] feat: add detect-missing-emergency-withdrawal rule and tests --- pr_body.md | 30 +++ .../detect-missing-emergency-withdrawal.ts | 250 ++++++++++++++++++ ...etect-missing-emergency-withdrawal.spec.ts | 197 ++++++++++++++ 3 files changed, 477 insertions(+) create mode 100644 pr_body.md create mode 100644 rules/security/emergency/detect-missing-emergency-withdrawal.ts create mode 100644 tests/rules/detect-missing-emergency-withdrawal.spec.ts diff --git a/pr_body.md b/pr_body.md new file mode 100644 index 0000000..b7cc756 --- /dev/null +++ b/pr_body.md @@ -0,0 +1,30 @@ +Closes #354 + +### Problem +Contracts that receive ETH or ERC-20 tokens—or that implement a pause/emergency mechanism—without any fund-recovery path risk permanently locking user funds if a critical bug is found, an admin key is lost, or the contract is paused indefinitely. + +### Solution +Implemented a new rule at `rules/security/emergency/detect-missing-emergency-withdrawal.ts` that statically analyzes Solidity source and flags contracts that: + +1. **Receive ETH** (`receive()`, `fallback()`, payable functions, `msg.value`) but expose no `withdraw`, `rescue`, `recover`, `emergencyExit`, `drain`, or `sweep` function and no `selfdestruct` call. +2. **Interact with ERC-20 tokens** (`IERC20`, `transferFrom`, `safeTransferFrom`, `.transfer(`, `.balanceOf(`) but expose no rescue/recovery function. +3. **Use a pause/emergency pattern** (`pause()`, `whenNotPaused`, `emergency`, `lockdown`) but provide no corresponding fund-recovery path. + +Each violation includes: +- The **contract name** and **line number** of the `contract` declaration. +- A human-readable **reason** explaining the risk. +- An actionable **suggestion** with a ready-to-use code template for the appropriate emergency flow. + +### Changes + +| File | Description | +|---|---| +| `rules/security/emergency/detect-missing-emergency-withdrawal.ts` | New rule implementation | +| `tests/rules/detect-missing-emergency-withdrawal.spec.ts` | Comprehensive tests covering all violation kinds, clean contracts, multiple contracts, and line-number accuracy | + +### Acceptance Criteria +- [x] Missing emergency withdrawals flagged (`eth-receiver-no-withdrawal`, `token-handler-no-withdrawal`, `pausable-no-withdrawal`) +- [x] Missing recovery methods detected +- [x] Emergency flow suggestions provided per violation kind +- [x] Clean contracts (with recovery methods or `selfdestruct`) are not flagged +- [x] Tests cover all scenarios diff --git a/rules/security/emergency/detect-missing-emergency-withdrawal.ts b/rules/security/emergency/detect-missing-emergency-withdrawal.ts new file mode 100644 index 0000000..423d3c0 --- /dev/null +++ b/rules/security/emergency/detect-missing-emergency-withdrawal.ts @@ -0,0 +1,250 @@ +/** + * Detect Missing Emergency Withdrawal Mechanisms (#354) + * + * Contracts that hold ETH or ERC-20 tokens without any emergency withdrawal + * or fund-recovery mechanism risk permanently locking user funds if the + * contract becomes paused, if access keys are lost, or if a critical bug + * is discovered. + * + * This rule flags contracts that: + * 1. Receive ETH (via `receive()`, `fallback()`, or `payable` functions) or + * interact with ERC-20 tokens but expose NO recovery function. + * 2. Have a `pause()` / `emergency` / `lockdown` pattern but no corresponding + * withdrawal or rescue path. + * + * A "recovery method" is any of: + * - A function whose name contains `withdraw`, `rescue`, `recover`, + * `emergencyExit`, `drain`, or `sweep`. + * - A `selfdestruct` call (deprecated but still used as an escape hatch). + * + * Suggestions surface actionable emergency-flow patterns the author can adopt. + */ + +export type EmergencyWithdrawalViolationKind = + | 'eth-receiver-no-withdrawal' + | 'token-handler-no-withdrawal' + | 'pausable-no-withdrawal'; + +export interface EmergencyWithdrawalViolation { + kind: EmergencyWithdrawalViolationKind; + contractName: string; + line: number; + snippet: string; + reason: string; + suggestion: string; +} + +export interface EmergencyWithdrawalResult { + detected: boolean; + violations: EmergencyWithdrawalViolation[]; + message: string; + suggestion: string; +} + +// ─── helpers ───────────────────────────────────────────────────────────────── + +const PRAGMA_RE = /pragma\s+solidity\s+([^;]+);/; + +function stripComments(code: string): string { + return code + .replace(/\/\/[^\n]*/g, '') + .replace(/\/\*[\s\S]*?\*\//g, ''); +} + +function lineAt(code: string, offset: number): number { + let line = 1; + for (let i = 0; i < offset; i++) { + if (code[i] === '\n') line++; + } + return line; +} + +// ─── patterns ──────────────────────────────────────────────────────────────── + +/** Matches `contract Name [is ...] {` — captures the contract name. */ +const CONTRACT_RE = /\bcontract\s+([A-Za-z_$][A-Za-z0-9_$]*)\s*(?:is\s+[^{]+)?\{/g; + +/** + * Patterns that indicate a contract receives / holds value. + * We look for each of these in the stripped source. + */ +const ETH_RECEIVER_PATTERNS: RegExp[] = [ + /\breceive\s*\(\s*\)\s*external\s+payable/, + /\bfallback\s*\(\s*\)\s*external\s+payable/, + /\bfunction\s+\w+\s*\([^)]*\)\s*(?:public|external)\s+payable/, + /\bmsg\.value\b/, +]; + +const TOKEN_HANDLER_PATTERNS: RegExp[] = [ + /\bIERC20\b/, + /\bsafeTransferFrom\b/, + /\btransferFrom\b/, + /\.transfer\s*\(/, + /\.balanceOf\s*\(/, +]; + +const PAUSABLE_PATTERNS: RegExp[] = [ + /\bpause\s*\(\s*\)/, + /\bemergency\b/i, + /\blockdown\b/i, + /\bwhenNotPaused\b/, +]; + +/** Names that constitute a recovery method. */ +const RECOVERY_NAME_RE = + /\bfunction\s+(withdraw|rescue|recover|emergencyExit|drain|sweep)\b/i; + +/** `selfdestruct` is also an escape hatch. */ +const SELFDESTRUCT_RE = /\bselfdestruct\b/; + +// ─── reason / suggestion maps ───────────────────────────────────────────── + +const REASON_MAP: Record = { + 'eth-receiver-no-withdrawal': + 'Contract receives or holds ETH but exposes no emergency withdrawal function. Funds may become permanently locked.', + 'token-handler-no-withdrawal': + 'Contract interacts with ERC-20 tokens but exposes no rescue or recovery function. Tokens may become permanently locked.', + 'pausable-no-withdrawal': + 'Contract has pause/emergency functionality but provides no corresponding fund-recovery path. Funds remain inaccessible if the contract is paused indefinitely.', +}; + +const SUGGESTION_MAP: Record = { + 'eth-receiver-no-withdrawal': [ + 'Add an owner-only (or role-gated) `emergencyWithdraw` function:', + ' function emergencyWithdraw(address payable to, uint256 amount) external onlyOwner {', + ' require(to != address(0), "zero address");', + ' (bool ok,) = to.call{value: amount}("");', + ' require(ok, "transfer failed");', + ' }', + 'Consider emitting an `EmergencyWithdrawal(address to, uint256 amount)` event.', + ].join('\n'), + 'token-handler-no-withdrawal': [ + 'Add an owner-only rescue function for stuck tokens:', + ' function rescueTokens(address token, address to, uint256 amount) external onlyOwner {', + ' IERC20(token).safeTransfer(to, amount);', + ' }', + 'Restrict to an admin role and emit a `TokensRescued(address token, address to, uint256 amount)` event.', + ].join('\n'), + 'pausable-no-withdrawal': [ + 'Pair your pause mechanism with an emergency withdrawal path, for example:', + ' function emergencyExit(address payable to) external onlyOwner whenPaused {', + ' uint256 bal = address(this).balance;', + ' (bool ok,) = to.call{value: bal}("");', + ' require(ok, "transfer failed");', + ' }', + 'This ensures funds are reachable even in a locked-down state.', + ].join('\n'), +}; + +// ─── per-contract extraction ────────────────────────────────────────────── + +/** + * Extract the body of a contract starting at `{` offset. + * Returns the raw (non-comment-stripped) body so line numbers remain accurate. + */ +function extractContractBody(code: string, openBraceOffset: number): string { + let depth = 0; + let i = openBraceOffset; + while (i < code.length) { + if (code[i] === '{') depth++; + else if (code[i] === '}') { + depth--; + if (depth === 0) return code.slice(openBraceOffset, i + 1); + } + i++; + } + return code.slice(openBraceOffset); +} + +function hasRecoveryMethod(body: string): boolean { + const stripped = stripComments(body); + return RECOVERY_NAME_RE.test(stripped) || SELFDESTRUCT_RE.test(stripped); +} + +function matchesAny(patterns: RegExp[], text: string): boolean { + return patterns.some((p) => p.test(text)); +} + +// ─── main export ───────────────────────────────────────────────────────────── + +export function detectMissingEmergencyWithdrawal( + code: string, +): EmergencyWithdrawalResult { + const violations: EmergencyWithdrawalViolation[] = []; + const stripped = stripComments(code); + + CONTRACT_RE.lastIndex = 0; + let contractMatch: RegExpExecArray | null; + + while ((contractMatch = CONTRACT_RE.exec(stripped)) !== null) { + const contractName = contractMatch[1]; + const bodyStart = stripped.indexOf('{', contractMatch.index + contractMatch[0].length - 1); + if (bodyStart === -1) continue; + + const body = extractContractBody(stripped, bodyStart); + const contractLine = lineAt(code, contractMatch.index); + const snippet = contractMatch[0].trim().split('\n')[0].trim(); + + const hasRecovery = hasRecoveryMethod(body); + if (hasRecovery) continue; // already protected + + const receivesEth = matchesAny(ETH_RECEIVER_PATTERNS, body); + const handlesTokens = matchesAny(TOKEN_HANDLER_PATTERNS, body); + const hasPause = matchesAny(PAUSABLE_PATTERNS, body); + + if (receivesEth) { + const kind: EmergencyWithdrawalViolationKind = 'eth-receiver-no-withdrawal'; + violations.push({ + kind, + contractName, + line: contractLine, + snippet, + reason: REASON_MAP[kind], + suggestion: SUGGESTION_MAP[kind], + }); + } else if (handlesTokens) { + const kind: EmergencyWithdrawalViolationKind = 'token-handler-no-withdrawal'; + violations.push({ + kind, + contractName, + line: contractLine, + snippet, + reason: REASON_MAP[kind], + suggestion: SUGGESTION_MAP[kind], + }); + } else if (hasPause) { + const kind: EmergencyWithdrawalViolationKind = 'pausable-no-withdrawal'; + violations.push({ + kind, + contractName, + line: contractLine, + snippet, + reason: REASON_MAP[kind], + suggestion: SUGGESTION_MAP[kind], + }); + } + } + + if (violations.length === 0) { + return { + detected: false, + violations: [], + message: 'No missing emergency withdrawal mechanisms detected.', + suggestion: '', + }; + } + + const summary = violations + .map((v) => `${v.contractName} (line ${v.line}): ${v.kind}`) + .join('; '); + + return { + detected: true, + violations, + message: `${violations.length} contract(s) lack emergency withdrawal mechanisms: ${summary}.`, + suggestion: + 'Add owner-gated or role-gated emergency withdrawal functions. ' + + 'Emit events for all fund movements. ' + + 'Consider a time-lock on large withdrawals and test the recovery path explicitly.', + }; +} diff --git a/tests/rules/detect-missing-emergency-withdrawal.spec.ts b/tests/rules/detect-missing-emergency-withdrawal.spec.ts new file mode 100644 index 0000000..a697685 --- /dev/null +++ b/tests/rules/detect-missing-emergency-withdrawal.spec.ts @@ -0,0 +1,197 @@ +import { detectMissingEmergencyWithdrawal } from '../../rules/security/emergency/detect-missing-emergency-withdrawal'; + +describe('detectMissingEmergencyWithdrawal', () => { + // ── ETH-receiver cases ──────────────────────────────────────────────────── + + it('flags a contract with receive() payable but no withdrawal function', () => { + const code = ` + pragma solidity ^0.8.0; + contract Vault { + receive() external payable {} + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(true); + expect(result.violations).toHaveLength(1); + expect(result.violations[0].kind).toBe('eth-receiver-no-withdrawal'); + expect(result.violations[0].contractName).toBe('Vault'); + expect(result.violations[0].suggestion).toContain('emergencyWithdraw'); + }); + + it('flags a contract with a payable function but no recovery path', () => { + const code = ` + pragma solidity ^0.8.0; + contract Deposit { + function deposit() external payable { + balances[msg.sender] += msg.value; + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(true); + expect(result.violations[0].kind).toBe('eth-receiver-no-withdrawal'); + }); + + it('does NOT flag a contract with receive() payable AND a withdraw function', () => { + const code = ` + pragma solidity ^0.8.0; + contract SafeVault { + receive() external payable {} + function withdraw(uint256 amount) external onlyOwner { + payable(msg.sender).transfer(amount); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(false); + }); + + it('does NOT flag when selfdestruct is used as an escape hatch', () => { + const code = ` + pragma solidity ^0.8.0; + contract LegacyVault { + receive() external payable {} + function kill(address payable to) external onlyOwner { + selfdestruct(to); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(false); + }); + + // ── Token-handler cases ─────────────────────────────────────────────────── + + it('flags a contract that uses IERC20 but has no rescue function', () => { + const code = ` + pragma solidity ^0.8.0; + contract TokenStaker { + IERC20 public token; + function stake(uint256 amount) external { + token.transferFrom(msg.sender, address(this), amount); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(true); + expect(result.violations[0].kind).toBe('token-handler-no-withdrawal'); + expect(result.violations[0].suggestion).toContain('rescueTokens'); + }); + + it('does NOT flag a token contract that exposes a rescue function', () => { + const code = ` + pragma solidity ^0.8.0; + contract SafeStaker { + IERC20 public token; + function stake(uint256 amount) external { + token.transferFrom(msg.sender, address(this), amount); + } + function rescueTokens(address tkn, address to, uint256 amt) external onlyOwner { + IERC20(tkn).transfer(to, amt); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(false); + }); + + // ── Pausable cases ──────────────────────────────────────────────────────── + + it('flags a pausable contract with no withdrawal path', () => { + const code = ` + pragma solidity ^0.8.0; + contract PausableProtocol { + bool public paused; + modifier whenNotPaused() { require(!paused); _; } + function pause() external onlyOwner { paused = true; } + function doSomething() external whenNotPaused {} + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(true); + expect(result.violations[0].kind).toBe('pausable-no-withdrawal'); + expect(result.violations[0].suggestion).toContain('emergencyExit'); + }); + + it('does NOT flag a pausable contract that also has emergencyExit', () => { + const code = ` + pragma solidity ^0.8.0; + contract SafePausable { + bool public paused; + modifier whenNotPaused() { require(!paused); _; } + function pause() external onlyOwner { paused = true; } + function emergencyExit(address payable to) external onlyOwner { + to.transfer(address(this).balance); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(false); + }); + + // ── Clean contract ──────────────────────────────────────────────────────── + + it('does NOT flag a contract that neither receives funds nor has pause', () => { + const code = ` + pragma solidity ^0.8.0; + contract Registry { + mapping(address => bool) public registered; + function register() external { + registered[msg.sender] = true; + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(false); + expect(result.violations).toHaveLength(0); + }); + + // ── Multiple contracts ──────────────────────────────────────────────────── + + it('reports violations for each affected contract separately', () => { + const code = ` + pragma solidity ^0.8.0; + contract A { + receive() external payable {} + } + contract B { + IERC20 token; + function deposit(uint256 amt) external { + token.transferFrom(msg.sender, address(this), amt); + } + } + contract C { + receive() external payable {} + function withdraw() external onlyOwner { + payable(msg.sender).transfer(address(this).balance); + } + } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.detected).toBe(true); + // A and B should be flagged; C should be clean + expect(result.violations).toHaveLength(2); + const names = result.violations.map((v) => v.contractName); + expect(names).toContain('A'); + expect(names).toContain('B'); + expect(names).not.toContain('C'); + }); + + // ── Line numbers ────────────────────────────────────────────────────────── + + it('provides a correct line number for the violation', () => { + const code = `pragma solidity ^0.8.0;\ncontract Vault {\n receive() external payable {}\n}`; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.violations[0].line).toBe(2); + }); + + // ── Result message ──────────────────────────────────────────────────────── + + it('includes contract names in the message', () => { + const code = ` + contract Risky { receive() external payable {} } + `; + const result = detectMissingEmergencyWithdrawal(code); + expect(result.message).toContain('Risky'); + }); +});