diff --git a/TODO.md b/TODO.md index 5a7cafc..83c6d00 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,24 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-20: A page asking which chain the wallet is on is told the chain the + user is actually on ([#317](https://git.eeqj.de/sneak/AutistMask/issues/317)). + `eth_chainId` and `net_version` answered from `currentNetwork()`, which reads + the module-level `state` singleton that nothing populates at module scope, so + a service worker revived by the page's own message answered out of + `DEFAULT_STATE` and reported mainnet `0x1`/`1` to a user on Sepolia — a dApp + building its interaction for the wrong chain. Both now answer from + `getState()`, the per-call detached storage read the other read handlers use, + rather than from the singleton: these two are reachable by any page on every + provider init, and mutating the shared singleton on that path would detach the + wallet objects an in-flight `backgroundRefresh()` is mutating. The read side + of the background was audited with it: the remaining singleton reads are the + chain switch, the transaction verification path and `backgroundRefresh`, which + each already load, and everything else answers from storage per call through + `getState()`. One stale read is left named but unfixed, outside this issue's + scope: `handleSendTransaction` builds its provider with no network name, so + `getProvider()` falls back to the same unloaded singleton for ethers' static + network hint. - 2026-08-20: The dApp approval screen no longer shows a token transfer it cannot scale as `0.0000` ([#306](https://git.eeqj.de/sneak/AutistMask/issues/306)). `decodeCalldata` diff --git a/src/background/index.js b/src/background/index.js index 58c349e..63abee1 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -3,7 +3,11 @@ // non-sensitive calls to the configured Ethereum JSON-RPC endpoint. const { DEFAULT_RPC_URL } = require("../shared/constants"); -const { SUPPORTED_CHAIN_IDS, networkByChainId } = require("../shared/networks"); +const { + SUPPORTED_CHAIN_IDS, + networkById, + networkByChainId, +} = require("../shared/networks"); const { onChainSwitch } = require("../shared/chainSwitch"); const { state, @@ -663,12 +667,28 @@ async function handleRpc(method, params, origin) { return { result: [] }; } - if (method === "eth_chainId") { - return { result: currentNetwork().chainId }; - } - - if (method === "net_version") { - return { result: currentNetwork().networkVersion }; + // Both answered from currentNetwork(), which reads the module-level state + // singleton, and nothing populates that at module scope. A worker revived + // by the page's own message therefore held DEFAULT_STATE and told a page + // it was on mainnet while the user was on Sepolia + // (https://git.eeqj.de/sneak/AutistMask/issues/317). + // + // Answered from getState() rather than by loading the singleton. Any page + // reaches these two — neither is gated on a connection, and the injected + // provider sends eth_chainId on every page load — and loadState() replaces + // state.wallets wholesale, which would detach the address objects an + // in-flight backgroundRefresh() is mutating across its network round trip, + // so its saveState() would persist the pre-refresh balances while still + // stamping lastBalanceRefresh. getState() is the detached per-call storage + // read the other read handlers here already use. + // networkById(undefined) falls back to mainnet, matching the default for a + // profile with no stored networkId. + if (method === "eth_chainId" || method === "net_version") { + const s = await getState(); + const net = networkById(s.networkId); + return { + result: method === "eth_chainId" ? net.chainId : net.networkVersion, + }; } if (method === "wallet_switchEthereumChain") { diff --git a/tests/coldWorkerChainId.test.js b/tests/coldWorkerChainId.test.js new file mode 100644 index 0000000..a95f87f --- /dev/null +++ b/tests/coldWorkerChainId.test.js @@ -0,0 +1,258 @@ +// What eth_chainId and net_version answer on a worker that has not loaded +// state yet. +// +// The MV3 service worker is terminated when idle and revived by the next +// message, and nothing loads state at module scope. Both methods answered from +// currentNetwork(), which reads the module-level `state` singleton, so a +// worker revived by the page's own message answered out of DEFAULT_STATE and +// told a page it was on mainnet while the user was on Sepolia +// (https://git.eeqj.de/sneak/AutistMask/issues/317). +// +// This file therefore uses the REAL state module and never calls loadState() +// itself: the handler has to answer from storage on its own. Same shape as +// tests/coldWorkerChainSwitch.test.js, which covers the write side. + +const { networkById } = require("../src/shared/networks"); + +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +const CONNECTED_ORIGIN = "https://dapp.example"; +const CONNECTED_HOSTNAME = "dapp.example"; +const UNKNOWN_ORIGIN = "https://stranger.example"; + +const MAINNET = networkById("mainnet"); +const SEPOLIA = networkById("sepolia"); + +const REFRESHED_BALANCE = "1.5"; + +function storedProfile(networkId) { + return { + hasWallet: true, + wallets: [ + { + name: "Wallet 1", + type: "hd", + addresses: [ + { address: ADDRESS, balance: "0", tokenBalances: [] }, + ], + }, + ], + activeAddress: ADDRESS, + networkId, + rpcUrl: networkById(networkId).defaultRpcUrl, + blockscoutUrl: networkById(networkId).defaultBlockscoutUrl, + allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] }, + deniedSites: {}, + trackedTokens: [], + }; +} + +async function settle() { + for (let i = 0; i < 50; i++) await Promise.resolve(); +} + +afterEach(() => { + delete global.chrome; +}); + +// Load the background worker with the real state module behind it, over a +// storage stub that keeps what is written. +// +// The stub structured-clones in both directions, as the real +// chrome.storage.local does. A stub that handed back the live stored object +// would alias it into whatever read it, so an in-place mutation of a detached +// copy would appear to have reached storage and this whole class of defect +// would be invisible here. +// +// opts.refreshBalances replaces the balances stub, so a test can hold a +// refresh open across a message. +function loadColdWorker(networkId, opts) { + jest.resetModules(); + + const options = opts || {}; + + jest.doMock("../src/shared/balances", () => ({ + getProvider: () => ({}), + refreshBalances: options.refreshBalances || jest.fn(async () => {}), + })); + jest.doMock("../src/shared/phishingDomains", () => ({ + isPhishingDomain: () => false, + })); + + let alarmHandlers = {}; + jest.doMock("../src/shared/alarms", () => ({ + BALANCE_REFRESH_ALARM: "balance", + BALANCE_REFRESH_PERIOD_MINUTES: 1, + ensureRecurringAlarms: jest.fn(async () => {}), + registerAlarmHandlers: jest.fn((handlers) => { + alarmHandlers = handlers; + }), + })); + + const store = { autistmask: storedProfile(networkId) }; + + let messageListener = null; + const set = jest.fn(async (items) => { + store.autistmask = structuredClone(items.autistmask); + }); + + global.chrome = { + storage: { + local: { + get: jest.fn(async () => structuredClone(store)), + set, + }, + }, + runtime: { + getURL: (path) => "chrome-extension://autistmask/" + path, + onMessage: { + addListener: (fn) => { + messageListener = fn; + }, + }, + onConnect: { addListener: () => {} }, + lastError: null, + }, + windows: { + getLastFocused: (cb) => cb(null), + create: (options, cb) => cb({ id: 1 }), + remove: (id, cb) => { + if (cb) cb(); + }, + onRemoved: { addListener: () => {} }, + }, + tabs: { + query: (queryInfo, cb) => cb([{ id: 1 }]), + sendMessage: (tabId, message, cb) => { + if (cb) cb(); + }, + }, + action: { setPopup: () => {} }, + }; + + require("../src/background/index"); + + async function rpc(method, origin) { + let result = null; + messageListener( + { type: "AUTISTMASK_RPC", method, params: [] }, + { origin: origin || CONNECTED_ORIGIN }, + (r) => { + result = r; + }, + ); + await settle(); + return result; + } + + return { + rpc, + persisted: () => store.autistmask, + storageSet: set, + fireBalanceAlarm: () => alarmHandlers.balance(), + }; +} + +describe("chain identity read by a worker that never loaded state", () => { + test("eth_chainId answers the stored chain, not the default", async () => { + // The first message this worker ever sees. Reading the unloaded + // singleton answers mainnet's 0x1 to a user who is on Sepolia. + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("eth_chainId")).toEqual({ + result: SEPOLIA.chainId, + }); + }); + + test("net_version answers the stored chain, not the default", async () => { + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("net_version")).toEqual({ + result: SEPOLIA.networkVersion, + }); + }); + + test("answers the stored chain to an origin that never connected", async () => { + // Neither method is gated on a connection, so the stale answer reached + // any page at all; the fixed answer has to as well. + const bg = loadColdWorker("sepolia"); + + expect(await bg.rpc("eth_chainId", UNKNOWN_ORIGIN)).toEqual({ + result: SEPOLIA.chainId, + }); + expect(await bg.rpc("net_version", UNKNOWN_ORIGIN)).toEqual({ + result: SEPOLIA.networkVersion, + }); + }); + + test("answers mainnet for a profile stored on mainnet", async () => { + // The default and the stored value agree here, so this case cannot + // catch the defect; it is what keeps the fix from being a swap. + const bg = loadColdWorker("mainnet"); + + expect(await bg.rpc("eth_chainId")).toEqual({ + result: MAINNET.chainId, + }); + expect(await bg.rpc("net_version")).toEqual({ + result: MAINNET.networkVersion, + }); + }); + + test("persists nothing: these are reads", async () => { + // The load must not turn a read into a write. saveState() persists + // every field of the singleton, and a read path that reached it would + // be the wipe https://git.eeqj.de/sneak/AutistMask/issues/316 fixed. + const bg = loadColdWorker("sepolia"); + + await bg.rpc("eth_chainId"); + await bg.rpc("net_version"); + + expect(bg.storageSet).not.toHaveBeenCalled(); + expect(bg.persisted()).toEqual(storedProfile("sepolia")); + }); + + test("a chain read arriving mid-refresh does not discard the refresh", async () => { + // Any page reaches these two methods, and the injected provider sends + // eth_chainId on every page load, so this overlap is ordinary traffic + // rather than a contrived race. + // + // backgroundRefresh() hands the singleton's wallets to + // refreshBalances(), which mutates those address objects in place once + // the network round trip resolves, and only then saves. Answering the + // page by calling loadState() would replace state.wallets mid-flight, + // so the refreshed balances would land on detached objects and the + // save that follows would persist the pre-refresh values — while still + // stamping lastBalanceRefresh, suppressing the redo. + let releaseRoundTrip; + const roundTrip = new Promise((resolve) => { + releaseRoundTrip = resolve; + }); + let refreshReachedNetwork; + const inFlight = new Promise((resolve) => { + refreshReachedNetwork = resolve; + }); + + const bg = loadColdWorker("sepolia", { + refreshBalances: async (wallets) => { + refreshReachedNetwork(); + await roundTrip; + // In place, on the objects handed in — as balances.js does. + wallets[0].addresses[0].balance = REFRESHED_BALANCE; + }, + }); + + const refresh = bg.fireBalanceAlarm(); + await inFlight; + + expect(await bg.rpc("eth_chainId", UNKNOWN_ORIGIN)).toEqual({ + result: SEPOLIA.chainId, + }); + + releaseRoundTrip(); + await refresh; + + expect(bg.persisted().wallets[0].addresses[0].balance).toBe( + REFRESHED_BALANCE, + ); + }); +});