diff --git a/.gitignore b/.gitignore index 925673f..cd1a27f 100644 --- a/.gitignore +++ b/.gitignore @@ -40,3 +40,7 @@ git-crypt-key.bin # Generated store/marketing screenshots (promote chosen shots to store-assets/ by hand) packages/extension/store-assets/screenshots/ + +# Extension store bundle — built in CI by .github/workflows/release.yml, never committed +# (a committed copy drifts from source; see Codex review on PR #8) +packages/extension/store-assets/*.zip diff --git a/docs/BUGS.md b/docs/BUGS.md index bedd9a6..7df6147 100644 Binary files a/docs/BUGS.md and b/docs/BUGS.md differ diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 5e27454..18143b9 100644 Binary files a/docs/DECISIONS.md and b/docs/DECISIONS.md differ diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index f00a023..829d024 100644 Binary files a/docs/ROADMAP.md and b/docs/ROADMAP.md differ diff --git a/docs/adr/ADR-016-trust-boundary-messageport-bridge.md b/docs/adr/ADR-016-trust-boundary-messageport-bridge.md new file mode 100644 index 0000000..76e48be Binary files /dev/null and b/docs/adr/ADR-016-trust-boundary-messageport-bridge.md differ diff --git a/docs/adr/README.md b/docs/adr/README.md index 75e8ac1..620e2de 100644 Binary files a/docs/adr/README.md and b/docs/adr/README.md differ diff --git a/packages/core/src/detectors.ts b/packages/core/src/detectors.ts index ff96929..508fcb5 100644 --- a/packages/core/src/detectors.ts +++ b/packages/core/src/detectors.ts @@ -230,27 +230,46 @@ export function detectTokenSelectors(instructions: Instruction[]): TokenSelector } export function detectEcrecover(instructions: Instruction[]): boolean { + // ecrecover is the precompile at address 0x01. Genuine detection requires the + // precompile address to be pushed before a CALL/STATICCALL. Two evidence levels: + // 1. PUSH20 0x00..01 — an unambiguous 20-byte address literal; nobody pushes + // this except to call the precompile, so it stands alone. + // 2. PUSH1 0x01 — indistinguishable from pushing the integer 1 (loop counters, + // booleans, lengths), so it counts ONLY when corroborated by a KECCAK256 in + // the same look-back window: real signature verification hashes the message + // immediately before the precompile call. (AUDIT-3: prevents an attacker from + // suppressing CRITICAL drainer warnings with a single stray PUSH1 0x01.) for (let i = 0; i < instructions.length; i++) { const instruction = instructions[i]; - // Check for both STATICCALL (0xFA) and CALL (0xF1) - older contracts use CALL for precompiles + // STATICCALL (0xFA) is the modern read-only precompile idiom; CALL (0xF1) is + // used by older contracts. Both are accepted with sufficient evidence. if (instruction.opcode === 'STATICCALL' || instruction.opcode === 'CALL') { const lookBackLimit = Math.max(0, i - LOOK_AHEAD.address); + let hasPush1Addr = false; + let hasKeccak = false; for (let j = i - 1; j >= lookBackLimit; j--) { const prevInstruction = instructions[j]; - if (prevInstruction.opcode === 'PUSH1' && prevInstruction.data) { - const value = prevInstruction.data[0]; - if (value === 0x01) { - return true; - } - } if (prevInstruction.opcode === 'PUSH20' && prevInstruction.data) { - const allZerosExceptLast = + const isPrecompileAddress = prevInstruction.data.slice(0, 19).every((b) => b === 0) && prevInstruction.data[19] === 0x01; - if (allZerosExceptLast) { + if (isPrecompileAddress) { return true; } } + if ( + prevInstruction.opcode === 'PUSH1' && + prevInstruction.data && + prevInstruction.data[0] === 0x01 + ) { + hasPush1Addr = true; + } + if (prevInstruction.opcode === 'KECCAK256') { + hasKeccak = true; + } + } + if (hasPush1Addr && hasKeccak) { + return true; } } } diff --git a/packages/core/tests/detectors.test.ts b/packages/core/tests/detectors.test.ts index 8ee4b1f..f0ec5a1 100644 --- a/packages/core/tests/detectors.test.ts +++ b/packages/core/tests/detectors.test.ts @@ -853,7 +853,7 @@ describe('detectTokenSelectors', () => { }); describe('detectEcrecover', () => { - it('detects ecrecover with STATICCALL + PUSH1 0x01', () => { + it('detects ecrecover with KECCAK256 + PUSH1 0x01 + STATICCALL', () => { const instructions = parseBytecode(AUTHORIZATION_CONTRACTS.withEcrecover); expect(detectEcrecover(instructions)).toBe(true); }); @@ -863,7 +863,7 @@ describe('detectEcrecover', () => { expect(detectEcrecover(instructions)).toBe(true); }); - it('detects ecrecover with CALL + PUSH1 0x01 (older contracts)', () => { + it('detects ecrecover with KECCAK256 + PUSH1 0x01 + CALL (older contracts)', () => { const instructions = parseBytecode(AUTHORIZATION_CONTRACTS.withEcrecoverCall); expect(detectEcrecover(instructions)).toBe(true); }); @@ -877,6 +877,25 @@ describe('detectEcrecover', () => { const instructions = parseBytecode(AUTHORIZATION_CONTRACTS.noAuth); expect(detectEcrecover(instructions)).toBe(false); }); + + // AUDIT-3: a bare `PUSH1 0x01` is ubiquitous (loop counters, booleans, lengths). + // It must NOT alone signal an ecrecover auth pattern, or an attacker can suppress + // CRITICAL drainer warnings by inserting one stray opcode before any CALL. + it('does NOT treat bare PUSH1 0x01 + CALL as ecrecover (drainer bypass)', () => { + const instructions = parseBytecode('0x6001f1'); // PUSH1 0x01; CALL + expect(detectEcrecover(instructions)).toBe(false); + }); + + it('does NOT treat bare PUSH1 0x01 + STATICCALL as ecrecover (no hash)', () => { + const instructions = parseBytecode('0x6001fa'); // PUSH1 0x01; STATICCALL + expect(detectEcrecover(instructions)).toBe(false); + }); + + it('detects real ecrecover: KECCAK256 + PUSH1 0x01 + STATICCALL', () => { + // PUSH1 0x00; PUSH1 0x00; KECCAK256; PUSH1 0x01; STATICCALL + const instructions = parseBytecode('0x60006000206001fa'); + expect(detectEcrecover(instructions)).toBe(true); + }); }); describe('detectMsgSenderCheck', () => { diff --git a/packages/core/tests/fixtures/contracts.ts b/packages/core/tests/fixtures/contracts.ts index 499fd2c..51491b5 100644 --- a/packages/core/tests/fixtures/contracts.ts +++ b/packages/core/tests/fixtures/contracts.ts @@ -155,16 +155,19 @@ export const TOKEN_TRANSFER_CONTRACTS = { noTokenSelectors: '0x6001600201', }; +// Real ecrecover hashes the message (KECCAK256) immediately before the precompile +// call, so a bare `PUSH1 0x01` is not sufficient evidence (AUDIT-3). These fixtures +// model the realistic shape: PUSH1 0x00; PUSH1 0x00; KECCAK256; PUSH1 0x01; (STATIC)CALL. export const AUTHORIZATION_CONTRACTS = { - withEcrecover: '0x6001fa', + withEcrecover: '0x60006000206001fa', withEcrecoverPush20: `0x73${'00'.repeat(19)}01fa`, - withEcrecoverCall: '0x6001f1', + withEcrecoverCall: '0x60006000206001f1', withEcrecoverCallPush20: `0x73${'00'.repeat(19)}01f1`, withMsgSenderCheck: '0x3360001014', withNonceTracking: '0x60005460016001015500', - withFullAuth: '0x6001fa3360001014600054600100015500', + withFullAuth: '0x60006000206001fa3360001014600054600100015500', noAuth: '0x63a9059cbb', - ecrecoverWithoutNonce: '0x63a9059cbb6001fa', + ecrecoverWithoutNonce: '0x63a9059cbb60006000206001fa', msgSenderWithoutEcrecover: '0x63a9059cbb3360001014', }; @@ -360,6 +363,6 @@ export const EXTCODEHASH_CONTRACTS = { export const DRAINER_PATTERNS = { infernoStyle: '0x63a22cb46573deadbeefdeadbeefdeadbeefdeadbeefdeadbeeef1', crimeEnjoyerWithToken: '0x3663a9059cbb60006000f1', - safeWalletPattern: '0x63a9059cbb6001fa600054600100015500', + safeWalletPattern: '0x63a9059cbb60006000206001fa600054600100015500', legitimateWithAuth: '0x63a9059cbb3360001014600054600100015500', }; diff --git a/packages/extension/CLAUDE.md b/packages/extension/CLAUDE.md index e829dfc..eab2cb2 100644 --- a/packages/extension/CLAUDE.md +++ b/packages/extension/CLAUDE.md @@ -31,7 +31,7 @@ window.ethereum.request intercepted ### Message Bridge (content.ts → background.ts) -**Transport**: CustomEvent + nonce channel (ADR-011). Content script generates `crypto.randomUUID()` nonce, passes via `data-testudo-nonce` attribute. Injected script reads nonce and creates channel with nonce-prefixed event names. Prevents response forgery by hostile dApps. +**Transport**: Private MessagePort bridge (ADR-016, supersedes ADR-011). `injected.js` is a declared MAIN-world content script; the ISOLATED content script creates a `MessageChannel` and transfers one port to it once at `document_start` (before any page script runs). All traffic then flows over a capability the page cannot observe, enumerate, or forge — no nonce. Order-resilient handshake (`ISO_READY`/`MAIN_READY`); fail-secure async init (no top-level throw). **Constants**: `src/utils/message-types.ts` — shared `MessageTypes` object (use instead of raw strings). @@ -94,7 +94,7 @@ src/ │ └── typed-data.ts # EIP-7702, permit, address extraction │ ├── services/ # I/O bridges -│ ├── channel.ts # CustomEvent + nonce channel (ADR-011) +│ ├── channel.ts # MessagePort bridge (ADR-016) │ ├── messaging.ts # IPC: sendTestudoRequest, requestAddressCheck, etc. │ └── deployer-lookup.ts # Blockscout API + viem RPC │ @@ -184,7 +184,7 @@ Located in `packages/extension/tests/`: | decoder/intent-builder.test.ts | 34 | All 8 context-specific intent builders, replay risk | | decoder/token-resolver.test.ts | 16 | Well-known lookup, RPC fallback, cache | | services/deployer-lookup.test.ts | 8 | Blockscout API, viem RPC, error handling | -| services/channel.test.ts | 11 | Channel isolation, nonce lifecycle, empty guard | +| services/channel.test.ts | 7 | MessagePort request/response correlation, timeout, handshake wiring | ```bash yarn workspace @testudo/extension run test diff --git a/packages/extension/manifest.json b/packages/extension/manifest.json index 3a0f197..36cf000 100644 --- a/packages/extension/manifest.json +++ b/packages/extension/manifest.json @@ -2,7 +2,7 @@ "manifest_version": 3, "name": "Testudo", "short_name": "Testudo", - "version": "0.3.0", + "version": "0.3.1", "description": "Antivirus for your Ethereum wallet. Analyzes contracts, approvals, and signatures before you sign — EIP-7702, permits, phishing, and more.", "homepage_url": "https://github.com/Lykhoyda/Testudo", "minimum_chrome_version": "137", @@ -45,12 +45,19 @@ "js": ["content.js"], "run_at": "document_start", "all_frames": true + }, + { + "matches": [""], + "js": ["injected.js"], + "run_at": "document_start", + "world": "MAIN", + "all_frames": true } ], "web_accessible_resources": [ { - "resources": ["injected.js", "fonts/*"], + "resources": ["fonts/*"], "matches": [""] } ], diff --git a/packages/extension/package.json b/packages/extension/package.json index 991293c..25e51d7 100644 --- a/packages/extension/package.json +++ b/packages/extension/package.json @@ -1,6 +1,6 @@ { "name": "@testudo/extension", - "version": "0.2.0", + "version": "0.3.1", "private": true, "type": "module", "scripts": { diff --git a/packages/extension/rolldown.config.ts b/packages/extension/rolldown.config.ts index 85da5b5..2a767db 100644 --- a/packages/extension/rolldown.config.ts +++ b/packages/extension/rolldown.config.ts @@ -78,8 +78,12 @@ const shared = { export default defineConfig([ { + // injected.js is a MAIN-world content script (ADR-016): it must be a single + // classic self-contained bundle (no ESM import/export, no chunks) because it + // runs under the page's CSP. IIFE guarantees that. input: 'src/injected.tsx', ...shared, + output: { ...shared.output, format: 'iife' as const }, }, { input: 'src/content.ts', diff --git a/packages/extension/src/analysis.ts b/packages/extension/src/analysis.ts index 2b14937..5741973 100644 --- a/packages/extension/src/analysis.ts +++ b/packages/extension/src/analysis.ts @@ -60,6 +60,19 @@ function cacheKey(address: string, chainId: number | undefined): string { return `${chain}:${address}`; } +/** + * Bytecode + deployer analysis currently runs only against Ethereum mainnet + * (DEFAULT_RPC + mainnet Blockscout). On any other chain those layers would + * query the WRONG chain — a false "clean" for an L2-only contract, or a bogus + * verdict from an unrelated mainnet contract at the same address (AUDIT-5). + * Until per-chain RPC + explorers exist we fail secure: skip the local layers + * off-mainnet and let the chain-aware threat API decide (the decision matrix + * preserves UNKNOWN, never a false clean). + */ +function supportsBytecodeAnalysis(chainId: number | undefined): boolean { + return chainId === undefined || chainId === DEFAULT_CHAIN_ID; +} + export function createAnalysisPipeline(deps: AnalysisDeps): AnalysisPipeline { const analysisCache = new Map(); const deployerCache = new Map(); @@ -186,17 +199,21 @@ export function createAnalysisPipeline(deps: AnalysisDeps): AnalysisPipeline { return result; } - // LAYER 1 + LAYER 2: Run API and Local Analysis in parallel + // LAYER 1 + LAYER 2: Run the chain-aware API and (mainnet-only) local + // analysis in parallel. Off-mainnet, bytecode + deployer analysis is + // skipped (fail-secure) so we never produce a wrong-chain verdict (AUDIT-5). const apiUrl = await getApiUrl(); + const runLocal = supportsBytecodeAnalysis(chainId); const settings = await deps.getSettings(); const rpcUrl = settings.rpcUrl || DEFAULT_RPC; - const client = deps.getOrCreateClient(rpcUrl); + const deployerKey = cacheKey(normalizedAddress, chainId); async function fetchDeployerStaticCached(addr: string): Promise { - const cached = deployerCache.get(addr); + const cached = deployerCache.get(deployerKey); if (cached) return cached; + const client = deps.getOrCreateClient(rpcUrl); const info = await deps.fetchDeployerStaticInfo(addr as Address, client); - if (info) deployerCache.set(addr, info); + if (info) deployerCache.set(deployerKey, info); return info; } @@ -205,11 +222,17 @@ export function createAnalysisPipeline(deps: AnalysisDeps): AnalysisPipeline { timeoutId = setTimeout(() => resolve('timeout'), ANALYSIS_TIMEOUT); }); + const localTasks = runLocal + ? [ + deps.analyzeContract(normalizedAddress as `0x${string}`, { rpcUrl }), + fetchDeployerStaticCached(normalizedAddress), + ] + : []; + const settled = await Promise.race([ Promise.allSettled([ deps.checkAddressThreat(normalizedAddress, { baseUrl: apiUrl, chainId }), - deps.analyzeContract(normalizedAddress as `0x${string}`, { rpcUrl }), - fetchDeployerStaticCached(normalizedAddress), + ...localTasks, ]).then((results) => { clearTimeout(timeoutId); return results; @@ -238,24 +261,40 @@ export function createAnalysisPipeline(deps: AnalysisDeps): AnalysisPipeline { return result; } - const [apiResult, localResult, deployerResult] = settled; - + const apiResult = settled[0]; const api = apiResult.status === 'fulfilled' - ? apiResult.value + ? (apiResult.value as ApiClientResult) : ({ success: false, error: 'Promise rejected' } as ApiClientResult); - let local = - localResult.status === 'fulfilled' - ? localResult.value - : ({ - risk: 'UNKNOWN', - threats: ['Local analysis failed'], - address: normalizedAddress, - blocked: false, - } as AnalysisResult); - - const deployerStatic = deployerResult.status === 'fulfilled' ? deployerResult.value : null; + let local: AnalysisResult; + let deployerStatic: DeployerStaticInfo | null = null; + + if (runLocal) { + const localResult = settled[1]; + const deployerResult = settled[2]; + local = + localResult?.status === 'fulfilled' + ? (localResult.value as AnalysisResult) + : ({ + risk: 'UNKNOWN', + threats: ['Local analysis failed'], + address: normalizedAddress, + blocked: false, + } as AnalysisResult); + deployerStatic = + deployerResult?.status === 'fulfilled' + ? (deployerResult.value as DeployerStaticInfo | null) + : null; + } else { + // Fail-secure: bytecode analysis was not run on this chain → UNKNOWN, never clean. + local = { + risk: 'UNKNOWN', + threats: [`Bytecode analysis unavailable on chain ${chainId}`], + address: normalizedAddress as `0x${string}`, + blocked: false, + } as AnalysisResult; + } // Merge deployer warnings into local result if (deployerStatic) { diff --git a/packages/extension/src/content.ts b/packages/extension/src/content.ts index 16ebe9b..3a57cce 100644 --- a/packages/extension/src/content.ts +++ b/packages/extension/src/content.ts @@ -1,15 +1,16 @@ /** - * CONTENT SCRIPT + * CONTENT SCRIPT (ISOLATED world) * - * Runs in isolated content script context. * Bridges communication between: - * - Injected script (page context) via CustomEvent channel (nonce-gated) - * - Background script (extension context) via chrome.runtime.sendMessage + * - The MAIN-world injected script, over a private MessagePort (ADR-016). + * injected.js is declared as a MAIN-world content script in the manifest, so + * this script no longer DOM-injects it. + * - The background service worker, via chrome.runtime.sendMessage. * - * Also responsible for injecting the injected.js script into the page. + * Also injects the warning-modal fonts and runs the navigation-time phishing check. */ -import { createChannel } from './services/channel'; +import { acceptIsolatedBridge, type BridgeMessage, type RequestReply } from './services/channel'; import { MessageTypes } from './utils/message-types'; // Layer A+ & B: Phishing domain check (top-level frame only) @@ -106,49 +107,11 @@ function injectFonts() { (document.head || document.documentElement).appendChild(style); } -// Inject the injected.js script into the page with a nonce for secure channel -function injectScript(): string { - const nonce = crypto.randomUUID(); - - // Set up handshake listener BEFORE injecting the module. - // The injected script will dispatch 'testudo-handshake' with a random token, - // and we respond on a token-specific event with the real channel nonce. - // All synchronous — the nonce never appears as a DOM attribute. (QA-005) - let handshakeComplete = false; - document.addEventListener( - 'testudo-handshake', - (e: Event) => { - if (handshakeComplete) return; - handshakeComplete = true; - const token = (e as CustomEvent).detail; - document.dispatchEvent( - new CustomEvent(`testudo-hs-${token}`, { - detail: nonce, - bubbles: false, - cancelable: false, - }), - ); - }, - { once: true }, - ); - - const script = document.createElement('script'); - script.src = chrome.runtime.getURL('injected.js'); - script.type = 'module'; - - // Insert at document_start to ensure we intercept before any dApp code runs - (document.head || document.documentElement).appendChild(script); - - script.onload = () => { - script.remove(); // Clean up after injection - }; - - return nonce; -} - -// Inject immediately +// injected.js is declared as a MAIN-world content script in the manifest (ADR-016), +// so the browser injects it at document_start — no DOM