Skip to content

fix(compliance-controller): reconcile compliance-status writes case-insensitively - #10060

Open
gomesalexandre wants to merge 1 commit into
MetaMask:mainfrom
gomesalexandre:fix_compliance_controller_casing_stale_cache
Open

fix(compliance-controller): reconcile compliance-status writes case-insensitively#10060
gomesalexandre wants to merge 1 commit into
MetaMask:mainfrom
gomesalexandre:fix_compliance_controller_casing_stale_cache

Conversation

@gomesalexandre

@gomesalexandre gomesalexandre commented Sep 1, 2026

Copy link
Copy Markdown

Summary

ComplianceController writes a wallet's compliance status to walletComplianceStatusMap keyed by the caller's raw address casing, but reads go through getWalletComplianceStatus (src/utils.ts, added by #8820) — which tries an exact match first, then falls back to a case-insensitive scan. That asymmetry means a second write for an already-cached wallet under a different casing creates a stale duplicate entry instead of updating the existing one, and a subsequent read can return either the fresh or the stale status for the same wallet depending on which casing was queried.

Rated as a stale-cache correctness bug on a sanctions-screening surface, not a demonstrated live OFAC-compliance bypass — see "Scope and severity" below.

The introducing commit is the whole argument

#8820 (merged, shipped in 2.1.0) created utils.ts and converted all four read call sites to the new case-insensitive helper. Its own PR body states the intent directly:

"Cached EVM wallet compliance statuses now exact-match first, then fall back to case-insensitive matching."

The diff contains not one line from either this.update(...) write block — git show 7645e56cb0/the PR diff confirms only the two catch-block fallback reads were touched. Casing-independence was the explicit goal; the write half needed the same treatment for it to actually hold across repeated writes under different casings.

Repro

// state starts empty
await controller.checkWalletCompliance(CHECKSUM_EVM_ADDRESS); // not blocked
// wallet gets sanctioned
await controller.checkWalletCompliance(LOWERCASE_EVM_ADDRESS); // blocked

Object.keys(controller.state.walletComplianceStatusMap)
// before this fix: [CHECKSUM_EVM_ADDRESS, LOWERCASE_EVM_ADDRESS]  <- two entries for one wallet
// after this fix:  [CHECKSUM_EVM_ADDRESS]                          <- one, casing preserved

selectIsWalletBlocked(CHECKSUM_EVM_ADDRESS)(controller.state)
// before this fix: false  <- stale, wrong
// after this fix:  true

The map is persist: true, usedInUi: true, so the stale duplicate survives app restarts and is what the UI actually reads. lastCheckedAt updates on every write regardless of casing, so a stale entry's presentation looks freshly-checked even while it reflects a pre-sanction verdict.

Fix

setWalletComplianceStatus (new, in utils.ts) collects every key in the map that case-insensitively matches the incoming address, writes the new status under the first (existing) one — preserving that key's casing so already-persisted state migrates for free without a breaking key-shape change — and deletes the rest. This also heals duplicate entries that already exist from before this fix ships (a real possibility, since the bug has been live since 2.1.0), not just prevents new ones from forming.

Both write sites in ComplianceController.ts (checkWalletCompliance, checkWalletsCompliance) now route through it. The batch path's loop mutates the same map object in place, so a second alias of the same address later in the same batch call correctly reconciles against an entry a prior iteration in that same call just wrote.

Scope and severity

The two-key/stale-read mechanism is proven end-to-end inside the package by the tests in this PR. What is not directly observed: a real client passing two different casings for the same wallet address in production. Both known consumers ship the affected version (extension ^2.1.0, mobile 2.1.0), and the extension's compliance selectors forward an arbitrary address: string with no normalization before calling into this controller, so nothing downstream closes the gap either — meaning the bug is reachable if a caller (or two callers, e.g. an EOA reference vs. a contract-derived checksum) ever supplies inconsistent casing for the same wallet, but that hasn't been directly confirmed happening today.

Tests

ComplianceController.test.ts:17-18 already defines fixtures under both casings, but every existing mixed-casing test pre-seeds state with one casing then reads with the other — none of them drive a write under a second casing after an initial write under a different casing, which is exactly the gap this bug lived in.

Added:

  • a single-check test proving a later write under a different casing reconciles into the existing entry (not a duplicate), with the existing key's casing preserved
  • a batch-check test proving the same for checkWalletsCompliance
  • a test proving pre-existing duplicate state (as if persisted from before this fix shipped) gets healed on the next write under either casing — this is the harder case, and an earlier draft of this fix that only reconciled the exact-match key vs. one other match missed it; a reviewer flagged the gap, I fixed it, and this test now catches the fix regressing back to that narrower version
  • a test proving two aliases of the same address requested within a single batch call reconcile to one entry with the later result winning

All new tests independently confirmed to fail with the exact predicted symptom against the pre-fix code, and pass after. Full package suite: 53/53 passing (was 49), 100% statement/branch/function/line coverage maintained. tsc --build and eslint clean on all changed files.

Dupe-check

gh issue/pr list --repo MetaMask/core --state all --search for getWalletComplianceStatus, walletComplianceStatusMap returns nothing beyond #8820 (the introducing PR) and merged PRs predating the helper. None of the ~20 currently open compliance-related PRs touch this package.

receipts

$ NODE_OPTIONS=--experimental-vm-modules yarn jest --config packages/compliance-controller/jest.config.js
Test Suites: 2 passed, 2 total
Tests:       53 passed, 53 total
Snapshots:   5 passed, 5 total
All files | 100% Stmts | 100% Branch | 100% Funcs | 100% Lines

$ npx eslint packages/compliance-controller/src/{utils,ComplianceController,ComplianceController.test}.ts
(clean, no output)

$ npx tsc --build tsconfig.build.json   # from packages/compliance-controller/
(clean, no output)

$ yarn workspace @metamask/compliance-controller run changelog:validate
(clean, no output)

Reviewed adversarially with Codex across two passes. First pass (real, run to completion) caught a genuine high-severity gap in the initial version: exact-match-first meant pre-existing stale duplicates from before this fix shipped weren't healed on write, only prevented going forward. Rewrote setWalletComplianceStatus to collect and reconcile all matching keys rather than stopping at the first, added the three tests above proving it, and got a clean second pass confirming the fix and asking only for the changelog entry (added).


Note

Medium Risk
Touches persisted sanctions-screening cache correctness; the change reduces stale/wrong blocked status risk rather than expanding attack surface, but wrong compliance data is safety-sensitive.

Overview
Fixes a stale-cache correctness bug where checkWalletCompliance and checkWalletsCompliance keyed persisted cache writes by the caller’s address casing while reads already matched EVM addresses case-insensitively. A second check for the same wallet under a different casing could leave an outdated entry alongside a fresh one, so UI/selectors might report the wrong blocked state.

Writes now go through new setWalletComplianceStatus, which updates the first existing case-insensitive match (keeping that key’s casing), removes any other duplicate keys, and collapses duplicate aliases within a single batch. Tests cover cross-casing updates, healing pre-existing duplicate persisted state, and batch reconciliation.

Reviewed by Cursor Bugbot for commit 93213b1. Bugbot is set up for automated code reviews on this repo. Configure here.

…nsensitively

ComplianceController wrote a wallet's compliance status to
walletComplianceStatusMap keyed by the caller's raw address casing, but
reads went through getWalletComplianceStatus (added in MetaMask#8820), which tries
an exact match first and falls back to a case-insensitive scan. Writing a
status for an already-cached wallet under a different casing therefore
created a second, stale entry instead of updating the existing one - and
depending on which casing a later read used, it could see either the fresh
or the stale status for the same wallet.

setWalletComplianceStatus now collects every key that case-insensitively
matches the incoming address, writes the new status under the first
(existing) one, and deletes the rest - healing duplicates already present
in persisted state, not just preventing new ones. Both write sites in
ComplianceController route through it.
@gomesalexandre
gomesalexandre marked this pull request as ready for review September 1, 2026 20:25
@gomesalexandre
gomesalexandre requested review from a team as code owners September 1, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant