diff --git a/TODO.md b/TODO.md index b68b5a0..78cc1a4 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: A page's request is credited only to the site the browser says + sent it ([#407](https://git.eeqj.de/sneak/AutistMask/issues/407)). Where the + browser does not give the sender's origin (Firefox before 126), the background + used the tab's page, so a frame from another site would have been treated as + the site embedding it, and with no tab it used an origin the page wrote into + the message. It now uses the URL of the frame that sent the message, and + refuses the request with code 4100 when the browser gives neither. The content + script no longer writes an origin into the message. + - 2026-10-04: `make test` takes 8-13s on the shared build host, down from 17-25s, measured in alternating runs before and after the change ([#428](https://git.eeqj.de/sneak/AutistMask/issues/428)). Each popup boot in diff --git a/src/background/index.js b/src/background/index.js index 5172cba..649b527 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -1288,18 +1288,29 @@ if (windowsNs && windowsNs.onRemoved) { // Listen for messages from content scripts and popup runtime.onMessage.addListener((msg, sender, sendResponse) => { if (msg.type === "AUTISTMASK_RPC") { - // Derive origin from trusted sender info to prevent origin spoofing. - // Chrome MV3 provides sender.origin; Firefox MV2 fallback uses sender.tab.url. - let trustedOrigin = msg.origin; // fallback only if sender info unavailable - if (sender.origin) { - trustedOrigin = sender.origin; - } else if (sender.tab && sender.tab.url) { + // The origin is the one the browser reports for the sender, never one + // the message carries. Firefox before 126 gives no sender.origin, so + // the origin of sender.url is used: the frame that sent the message, + // not the tab's page, which may be another site embedding that + // frame. With neither, the request is refused. + let trustedOrigin = sender.origin; + if (!trustedOrigin && sender.url) { try { - trustedOrigin = new URL(sender.tab.url).origin; + trustedOrigin = new URL(sender.url).origin; } catch { - // keep fallback + // an unparseable URL leaves the origin unknown } } + if (!trustedOrigin) { + sendResponse({ + error: { + code: 4100, + message: + "The wallet could not tell which site sent this request.", + }, + }); + return false; + } handleRpc(msg.method, msg.params, trustedOrigin) .then((response) => { sendResponse(response); diff --git a/src/content/index.js b/src/content/index.js index 4c71f72..cc1689f 100644 --- a/src/content/index.js +++ b/src/content/index.js @@ -30,7 +30,6 @@ window.addEventListener("message", (event) => { id, method, params, - origin: location.origin, }) .then((response) => { if (response) { diff --git a/tests/rpcOrigin.test.js b/tests/rpcOrigin.test.js new file mode 100644 index 0000000..cecfdab --- /dev/null +++ b/tests/rpcOrigin.test.js @@ -0,0 +1,147 @@ +// Which site a page's request is attributed to. +// +// The background takes a request's origin from what the browser says sent the +// message: sender.origin, or on Firefox before 126, which has no +// sender.origin, the origin of sender.url — the frame that sent it. It used to +// fall back to the tab's page and then to an origin the message itself +// carried, so a request from a frame was credited to the site embedding it, +// and a request the browser said nothing about was credited to whatever the +// page wrote (https://git.eeqj.de/sneak/AutistMask/issues/407). +// +// Every sender here lacks sender.origin, as on old Firefox. The connection +// check on eth_accounts is what shows which site a request was credited to. + +const { makeStorageStub } = require("./support/storageStub"); + +const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; + +// The site the persisted state has connected, and one it has never heard of. +const CONNECTED_ORIGIN = "https://dapp.example"; +const CONNECTED_HOSTNAME = "dapp.example"; +const STRANGER_ORIGIN = "https://stranger.example"; + +async function settle() { + for (let i = 0; i < 50; i++) await Promise.resolve(); +} + +afterEach(() => { + delete global.chrome; +}); + +function loadBackground() { + jest.resetModules(); + + jest.doMock("../src/shared/balances", () => ({ + getProvider: () => ({}), + refreshBalances: jest.fn(async () => {}), + })); + jest.doMock("../src/shared/phishingDomains", () => ({ + isPhishingDomain: () => false, + })); + jest.doMock("../src/shared/alarms", () => ({ + BALANCE_REFRESH_ALARM: "balance", + BALANCE_REFRESH_PERIOD_MINUTES: 1, + ensureRecurringAlarms: jest.fn(async () => {}), + registerAlarmHandlers: jest.fn(), + })); + + const storage = makeStorageStub({ + autistmask: { + networkId: "mainnet", + wallets: [ + { + name: "Wallet 1", + type: "hd", + addresses: [ + { address: ADDRESS, balance: "0", tokenBalances: [] }, + ], + }, + ], + activeAddress: ADDRESS, + allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] }, + deniedSites: {}, + }, + }); + + let messageListener = null; + global.chrome = { + storage, + runtime: { + getURL: (path) => "chrome-extension://autistmask/" + path, + onMessage: { + addListener: (fn) => { + messageListener = fn; + }, + }, + onConnect: { addListener: () => {} }, + lastError: null, + }, + windows: { onRemoved: { addListener: () => {} } }, + action: { setPopup: () => {} }, + }; + + require("../src/background/index"); + + // Ask for eth_accounts. `claimedOrigin` is an origin written into the + // message, as the content script used to send. + return async function accounts(sender, claimedOrigin) { + let result = null; + messageListener( + { + type: "AUTISTMASK_RPC", + method: "eth_accounts", + params: [], + origin: claimedOrigin, + }, + sender, + (r) => { + result = r; + }, + ); + await settle(); + return result; + }; +} + +describe("a request is attributed to the frame that sent it", () => { + test("a stranger's frame on a connected site gets no address", async () => { + const accounts = loadBackground(); + + const result = await accounts({ + url: STRANGER_ORIGIN + "/frame.html", + tab: { url: CONNECTED_ORIGIN + "/" }, + }); + + expect(result).toEqual({ result: [] }); + }); + + test("a connected site's frame on a stranger's page gets the address", async () => { + const accounts = loadBackground(); + + const result = await accounts({ + url: CONNECTED_ORIGIN + "/frame.html", + tab: { url: STRANGER_ORIGIN + "/" }, + }); + + expect(result).toEqual({ result: [ADDRESS] }); + }); +}); + +describe("a request the browser does not say the sender of", () => { + test("is refused, whatever the tab or the message says", async () => { + const accounts = loadBackground(); + + const result = await accounts( + { tab: { url: CONNECTED_ORIGIN + "/" } }, + CONNECTED_ORIGIN, + ); + + expect(result).toEqual({ + error: { + code: 4100, + message: + "The wallet could not tell which site sent this request.", + }, + }); + }); +});