fix: resolve all 11 MEDIUM audit findings (AUDIT-7/8/9/10/12/13/14/15/16/17/18)#13
Conversation
…n deployer merge (AUDIT-9) AUDIT-10: benign proxy/wallet capabilities (proxy/diamond/minimal-proxy/multicall, and proxy-mitigated delegatecall) at MEDIUM no longer count toward the 2+→block escalation, so legitimate upgradeable proxies aren't auto-blocked. Genuine MEDIUM combos (timestamp+coinbase, CREATE2+chainid) and any HIGH still block. 5 tests that encoded the old posture updated (owner-approved relaxation). AUDIT-9: in the deployer-warning merge, an incomplete (UNKNOWN) bytecode result is preserved unless the deployer warning escalates to a blocking HIGH/CRITICAL — a deployer MEDIUM no longer masks 'we could not inspect this contract' as a confident verdict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (AUDIT-7)
AUDIT-8: bound each three-layer task by the global timeout independently instead
of racing allSettled-vs-timeout and discarding everything. A completed API verdict
(e.g. 'malicious') now survives a slow bytecode fetch and flows through the
decision matrix — a timeout is never a blind clean allow; it BLOCKS if the API
flagged the address.
AUDIT-7: in the delegation-only analysis path, an authorization target with no
deployed code (counterfactual/CREATE2) is elevated from passive UNKNOWN to a
blocking HIGH warning ('no_deployed_code') so the user must confirm — closing the
deploy-after-sign gap. (Owner-approved behavior change.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/14/15/16) Cluster B (decoder/display): - AUDIT-13: formatTokenAmount treats only the max-uint sentinels as "Unlimited" (consistent with isUnlimitedValue). The old 10^28-on-raw heuristic mislabelled large-but-finite approvals (e.g. 10B of an 18-decimal token) as unlimited. - AUDIT-15: when decimals are unavailable, render raw base units with an explicit "(raw, decimals unknown)" caveat instead of a bare integer that reads as a token count. buildPermitIntent now routes through formatTokenAmount instead of its ad-hoc "(raw)" branch. - AUDIT-16: bound attacker-controlled `decimals` (0..36); out-of-range values fall back to the raw-base-units caveat, and a tiny nonzero value renders as "<0.000001" rather than a misleading "0.000000". - AUDIT-14: don't cache a fully-failed (all-null) token resolution — a transient RPC error would otherwise poison every later lookup with no TTL. Tests: +7 new, 3 existing updated. Extension 393/393 pass, build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ilience (AUDIT-12/17/18) Cluster C (interception/parsers/bg): - AUDIT-12: approve()/setApprovalForAll() interception now threat-checks the called token/collection contract (tx `to`) in parallel with the spender/operator and blocks if either is known-malicious. A malicious token contract is itself a threat vector, independent of the allowance grantee. - AUDIT-17: blind-signature hex decode uses UTF-8 (TextDecoder), keeps clean multi-byte text whole, and extracts the printable substring from mixed binary/text payloads so embedded phishing copy reaches detectPhishingPatterns instead of the scanner seeing raw hex. +2 tests. - AUDIT-18: TESTUDO_PHISHING_OVERRIDE wraps the rule update in try/catch (always responds, so blocked.html never hangs) and includes removeRuleIds:[ruleId] so a repeat override atomically replaces the per-tab rule instead of throwing. Docs: BUGS.md statuses, DECISIONS.md (AUDIT-10 posture / unified amount-display / dual-contract-check), ROADMAP.md. Verification: core 398/398, extension 395/395, build clean, biome clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pure formatting (biome) on the cluster A timeout/no-code-elevation edits in analysis.ts and analysis.test.ts. No behavior change; 42/42 analysis tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0.3.2 ships the MEDIUM audit-findings sweep (AUDIT-7/8/9/10/12/13/14/15/16/17/18) on top of the v0.3.1 CRITICAL/HIGH security fixes. manifest.json and package.json bumped in lockstep so the release.yml version guard and the v0.3.2 tag agree. CWS reserves accepted versions forever, so 0.3.1 is retired in favor of 0.3.2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 241da3f8e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const api: ApiClientResult = apiB.ok | ||
| ? (apiB.value as ApiClientResult) | ||
| : ({ success: false, error: 'API timeout or unavailable' } as ApiClientResult); |
There was a problem hiding this comment.
When the API lookup is the task that hits the shared deadline while local bytecode analysis has already returned LOW, this converts the timeout into the generic success:false value; applyDecisionMatrix then takes its API unavailable, local clean branch and returns LOW, so an EIP-7702 delegation can proceed without even the UNKNOWN notice even though threat intelligence never answered. Before this change, a global timeout returned UNKNOWN, so please keep an API-timeout/incomplete signal or force the combined result to UNKNOWN unless the API has actually answered clean/malicious.
Useful? React with 👍 / 👎.
| const [analysis, tokenAnalysis] = await Promise.all([ | ||
| requestAddressCheck(approvalInfo.spender, chainId), | ||
| requestAddressCheck(approvalInfo.tokenAddress, chainId), | ||
| ]); |
There was a problem hiding this comment.
For ERC-20 approvals, if either of these two address checks rejects or times out, Promise.all throws and the outer transaction catch falls back to originalRequest(args), discarding any completed HIGH/CRITICAL verdict from the other lookup. That means a malicious spender can be allowed whenever the new token-contract check fails independently (and vice versa); use per-lookup catches or Promise.allSettled so a completed dangerous verdict still blocks.
Useful? React with 👍 / 👎.
1 participant
Resolves all actionable MEDIUM findings from the 2026-06-07 multi-agent audit, across 3 TDD clusters on top of the already-shipped CRITICAL/HIGH fixes.
Cluster A — risk-scoring (
@testudo/core,analysis.ts)no_deployed_code) after the decision matrix, closing the deploy-after-sign drain path.UNKNOWN; only a derived CRITICAL/HIGH escalates, so analysis failure is no longer masked as false confidence.Cluster B — decoder/display
formatTokenAmountlabels only the max-uint sentinels "Unlimited" (consistent withisUnlimitedValue); the old10^28-on-raw heuristic that mislabelled large-but-finite approvals is gone.(raw, decimals unknown)caveat instead of a bare integer;buildPermitIntentroutes throughformatTokenAmount.decimalsbounded to0..36(out-of-range falls back to the caveat); a tiny nonzero value renders<0.000001, never a misleading0.000000.Cluster C — interception/parsers/bg
approve()/setApprovalForAll()interception now threat-checks the called token/collection contract (to) in parallel with the spender/operator and blocks if either is known-malicious.TextDecoder), keeps clean multi-byte text whole, and extracts the printable substring from mixed binary/text so embedded phishing copy reachesdetectPhishingPatternsinstead of the scanner seeing raw hex.TESTUDO_PHISHING_OVERRIDEwraps the rule update in try/catch (always responds, soblocked.htmlnever hangs) and includesremoveRuleIds:[ruleId]so a repeat override atomically replaces the per-tab rule.Testing
injected.tsx) and AUDIT-18 (background.ts) are wiring changes in the page-context orchestrator / SW message router, which have no unit-test harness in this repo — their underlying logic is covered byanalysis.test.ts/decision-matrix.test.tsand the Synpress e2e gate. Verified via build + biome + e2e, not new unit tests.Docs
docs/BUGS.md— all 11 statuses → fixed.docs/DECISIONS.md— posture (AUDIT-10), unified amount-display contract (AUDIT-13/15/16), dual-contract-check (AUDIT-12).docs/ROADMAP.md— MEDIUM sweep entry + refreshed test counts.🤖 Generated with Claude Code