feat: vendor and censor the phishing blocklist at build time (closes #219)
The blocklist URL in shipped code named a competitor and pointed at a moving ref, and the extension re-fetched from it every 24 hours, which also meant a third party decided what this wallet warns about. All of that is gone. script/vendor-blocklist fetches upstream at a pinned commit, verifies the sha256 of the bytes that commit serves, and writes src/shared/phishingBlocklist.json. It is build-time tooling, never shipped, and the one place in the repo that names the upstream project; a source reference nobody can verify is not a source reference. The artifact stores truncated sha256 digests rather than domain names. That is what censors it: the previous file contained the competitor's name 6,475 times, as phishing domains impersonating them, and not one of those domains is dropped. It also makes lookups a binary search over a fixed-width string, so nothing is built at module load — which matters on MV3, where the worker re-evaluates the module on every wake — and takes the file from 8.7 MB to 1.7 MB. script/check-censored enforces the rest: it reads the name out of the vendoring script rather than repeating it, and fails on any occurrence in the working tree or under dist/ that is not one of the two literals shipped code cannot avoid. It runs in make check, which inspects dist/ when there is one and says loudly when there is not, and again with --require-dist at the end of every make build. Removing the runtime fetch retires the delta, the extension-storage persistence and the 24-hour alarm from #158. A retired alarm is now cleared rather than left waking the worker forever on installs that already have it. The e2e suite drives the warning end to end from a real blocklisted origin served as a real http(s) site, with a control asserting the banner stays hidden for one that is not listed. Its service-worker interception canary needed a new anchor, since the startup fetch it used to watch for no longer happens: it now wakes the worker with a message and asks it for one throwaway fetch. LICENSE no longer cites a repository that returns 404. eslint.config.js gains one block: script/lib/ holds node programs the shell entrypoints call, and without it they lint with no globals at all.
This commit is contained in:
@@ -13,7 +13,7 @@ const os = require("os");
|
||||
const path = require("path");
|
||||
|
||||
const { chromium } = require("playwright-core");
|
||||
const { installNetworkStubs } = require("./network");
|
||||
const { installNetworkStubs, WORKER_PROBE_URL } = require("./network");
|
||||
|
||||
const REPO_ROOT = path.resolve(__dirname, "..", "..");
|
||||
const EXT_PATH = path.join(REPO_ROOT, "dist", "chrome");
|
||||
@@ -129,42 +129,109 @@ function attachErrorListeners(ctx, errors) {
|
||||
// if it ever stops being.
|
||||
}
|
||||
|
||||
function sleep(ms) {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
// The most recently seen background worker, waiting for one if none has
|
||||
// appeared yet. Most recent rather than first: Chrome stops an idle MV3
|
||||
// worker and starts a fresh one on the next event, and a handle to a
|
||||
// stopped worker cannot be evaluated in.
|
||||
async function serviceWorker(ctx) {
|
||||
const [existing] = ctx.serviceWorkers();
|
||||
if (existing) return existing;
|
||||
const workers = ctx.serviceWorkers();
|
||||
const latest = workers[workers.length - 1];
|
||||
if (latest) return latest;
|
||||
return ctx.waitForEvent("serviceworker", { timeout: 30000 });
|
||||
}
|
||||
|
||||
// How long to wait for the background worker's first outbound request.
|
||||
//
|
||||
// The margin that actually decides whether this check is sound is not
|
||||
// this timeout — it is whether the route handler is installed before the
|
||||
// worker fetches. Measured over several runs: route installation
|
||||
// completes 11-23ms after the context comes up, and the worker's
|
||||
// blocklist fetch arrives 525-883ms after that, so the route wins by
|
||||
// roughly 25-50x. This 30s figure is only slack for a loaded machine on
|
||||
// top of that; losing the race fails the run rather than passing it
|
||||
// quietly, which was verified by forcing a 3s delay before route
|
||||
// installation.
|
||||
// How long to wait for the probe request the worker is asked to make.
|
||||
const WORKER_TRAFFIC_TIMEOUT_MS = 30000;
|
||||
|
||||
// ctx.route() only sees service-worker requests when Playwright runs with
|
||||
// PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1, which script/test-e2e
|
||||
// sets. Without it the worker's traffic — notably the phishing blocklist
|
||||
// fetch src/background/index.js issues at startup — goes to the real
|
||||
// internet, and nothing says so, because src/shared/phishingDomains.js
|
||||
// swallows fetch failures. A harness whose isolation can lapse in silence
|
||||
// is worthless, so this does not take the flag on trust: the background
|
||||
// worker's own startup fetch has to show up in the route handler, or the
|
||||
// suite refuses to run.
|
||||
// sets. Without it every fetch the background worker makes goes to the
|
||||
// real internet and nothing says so. A harness whose isolation can lapse
|
||||
// in silence is worthless, so this does not take the flag on trust: a
|
||||
// request the worker itself issues has to show up in the route handler,
|
||||
// or the suite refuses to run.
|
||||
//
|
||||
// Deliberately NOT a synthetic probe fetched through worker.evaluate():
|
||||
// evaluating in an extension worker this early kills it (the call fails
|
||||
// with "Target page, context or browser has been closed" and the worker
|
||||
// disappears), which would break the very thing being measured. Observing
|
||||
// traffic the extension already generates costs nothing and cannot
|
||||
// perturb it.
|
||||
async function assertWorkerTrafficIntercepted(stubs) {
|
||||
// The anchor is a probe the harness asks the worker for, not traffic the
|
||||
// extension generates on its own. It used to be the phishing blocklist
|
||||
// fetch src/background/index.js issued at startup; that fetch is gone —
|
||||
// the blocklist is vendored at build time and the extension contacts
|
||||
// nobody when it starts — so there is no longer any startup traffic to
|
||||
// observe and the check generates its own.
|
||||
//
|
||||
// Evaluating in the worker straight after launch does not work, and that
|
||||
// is not a stale observation: it was tried again here and failed with
|
||||
// "Target page, context or browser has been closed" on the first run.
|
||||
// Chrome stops the freshly registered worker as soon as it has nothing to
|
||||
// do, and the extension no longer gives it anything to do — which is the
|
||||
// same change that removed the old anchor. So the probe wakes the worker
|
||||
// before it evaluates in it, by sending it a message from an extension
|
||||
// page and waiting for the reply: delivering a message is what starts a
|
||||
// stopped worker, and a worker that has just answered one is alive.
|
||||
// The evaluated fetch is not awaited, so nothing in the worker is held
|
||||
// open by the probe either.
|
||||
async function wakeWorker(ctx) {
|
||||
const sw = await serviceWorker(ctx);
|
||||
const extensionId = new URL(sw.url()).host;
|
||||
const page = await ctx.newPage();
|
||||
try {
|
||||
await page.goto(
|
||||
"chrome-extension://" + extensionId + "/src/popup/index.html",
|
||||
);
|
||||
// eth_chainId is answered from local state: it wakes the worker
|
||||
// and changes nothing.
|
||||
await page.evaluate(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
chrome.runtime.sendMessage(
|
||||
{
|
||||
type: "AUTISTMASK_RPC",
|
||||
method: "eth_chainId",
|
||||
params: [],
|
||||
},
|
||||
() => resolve(null),
|
||||
);
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
await page.close();
|
||||
}
|
||||
}
|
||||
|
||||
async function probeFromWorker(ctx, url) {
|
||||
let lastError = null;
|
||||
for (let attempt = 0; attempt < 5; attempt++) {
|
||||
try {
|
||||
await wakeWorker(ctx);
|
||||
const sw = await serviceWorker(ctx);
|
||||
await sw.evaluate((u) => {
|
||||
// Deliberately not awaited and never rejected: what is
|
||||
// being observed is that the request reaches the route
|
||||
// handler, and an unhandled rejection in the worker would
|
||||
// be collected as a suite error if it did not.
|
||||
fetch(u).catch(() => {});
|
||||
}, url);
|
||||
return;
|
||||
} catch (e) {
|
||||
lastError = e;
|
||||
await sleep(500);
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
"could not ask the background worker to fetch " +
|
||||
url +
|
||||
", so service-worker interception was never tested. Last " +
|
||||
"error: " +
|
||||
(lastError && lastError.message),
|
||||
);
|
||||
}
|
||||
|
||||
async function assertWorkerTrafficIntercepted(ctx, stubs) {
|
||||
await probeFromWorker(ctx, WORKER_PROBE_URL);
|
||||
|
||||
const seen = await stubs.waitForServiceWorkerTraffic(
|
||||
WORKER_TRAFFIC_TIMEOUT_MS,
|
||||
);
|
||||
@@ -177,19 +244,16 @@ async function assertWorkerTrafficIntercepted(stubs) {
|
||||
throw new Error(
|
||||
"observed no service-worker request in the route handler within " +
|
||||
WORKER_TRAFFIC_TIMEOUT_MS +
|
||||
"ms. Under working interception the background worker's " +
|
||||
"startup blocklist fetch (src/background/index.js) reaches the " +
|
||||
"handler about half a second after the route is installed. " +
|
||||
"Two causes are plausible and this check cannot distinguish " +
|
||||
"ms, although the background worker was asked to fetch " +
|
||||
WORKER_PROBE_URL +
|
||||
". Two causes are plausible and this check cannot distinguish " +
|
||||
"them: (1) service-worker interception is not in effect, so " +
|
||||
"that traffic went to the real internet unobserved — the suite " +
|
||||
"that request went to the real internet unobserved — the suite " +
|
||||
"must be run through script/test-e2e, which sets " +
|
||||
"PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1, and a " +
|
||||
"Playwright upgrade may have dropped or renamed that flag; " +
|
||||
"(2) no worker request was made in the first place — the route " +
|
||||
"lost the startup race, or the worker no longer fetches at " +
|
||||
"startup, in which case this check needs a new anchor because " +
|
||||
"there is no longer any worker traffic to observe. Either way " +
|
||||
"(2) the probe never ran, because the worker was torn down " +
|
||||
"between being handed over and being evaluated in. Either way " +
|
||||
"the fix is a replacement mechanism or an honest downgrade of " +
|
||||
"the isolation claims in tests/e2e/network.js and README.md — " +
|
||||
"not deleting this check",
|
||||
@@ -241,7 +305,7 @@ async function launch(routeOpts) {
|
||||
routeOpts.report = (text) => errors.record("network", text);
|
||||
const stubs = await installNetworkStubs(ctx, routeOpts);
|
||||
|
||||
await assertWorkerTrafficIntercepted(stubs);
|
||||
await assertWorkerTrafficIntercepted(ctx, stubs);
|
||||
|
||||
// The extension id is derived from the unpacked path, so it
|
||||
// changes and must never be hardcoded. It is the host part of the
|
||||
|
||||
@@ -9,13 +9,12 @@
|
||||
//
|
||||
// Service-worker coverage is not free: ctx.route() only sees worker
|
||||
// traffic when PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 is set in
|
||||
// the environment, which script/test-e2e does. Without it the phishing
|
||||
// blocklist fetch that src/background/index.js issues at worker startup
|
||||
// silently reaches raw.githubusercontent.com on the open internet, and
|
||||
// src/shared/phishingDomains.js swallows the failure so nothing surfaces
|
||||
// it. That is not left to trust: waitForServiceWorkerTraffic() below
|
||||
// backs the launch-time canary in harness.js, which fails the entire
|
||||
// suite if worker requests stop being visible here.
|
||||
// the environment, which script/test-e2e does. Without it every fetch the
|
||||
// MV3 background worker makes — the JSON-RPC calls behind every approval
|
||||
// in this suite among them — goes to the real internet unobserved. That is
|
||||
// not left to trust: waitForServiceWorkerTraffic() below backs the
|
||||
// launch-time canary in harness.js, which fails the entire suite if worker
|
||||
// requests stop being visible here.
|
||||
//
|
||||
// Anything not explicitly stubbed here is aborted AND reported to the
|
||||
// error collector, so a newly added outbound call shows up as a test
|
||||
@@ -103,6 +102,22 @@ function word(value) {
|
||||
const DAPP_ORIGIN = "https://dapp.e2e.test";
|
||||
const DAPP_URL = DAPP_ORIGIN + "/";
|
||||
|
||||
// The same page, served from a hostname that is on the vendored phishing
|
||||
// blocklist, so the phishing warning can be driven end to end against the real
|
||||
// list rather than a stub of it. It is a live entry at the pinned upstream
|
||||
// commit; upstream prunes, so a re-vendoring run that retires it turns the
|
||||
// phishing test red, and the fix is a current entry, not a weaker assertion.
|
||||
const PHISHING_DAPP_ORIGIN = "https://myetheywallet.com";
|
||||
const PHISHING_DAPP_URL = PHISHING_DAPP_ORIGIN + "/";
|
||||
|
||||
// A request the harness asks the background service worker to make, purely so
|
||||
// that worker interception can be proved before any test runs. Nothing in the
|
||||
// extension fetches at startup any more — the blocklist is vendored at build
|
||||
// time — so the canary in harness.js has no product traffic to anchor on and
|
||||
// generates its own. See assertWorkerTrafficIntercepted().
|
||||
const WORKER_PROBE_ORIGIN = "https://worker-probe.e2e.test";
|
||||
const WORKER_PROBE_URL = WORKER_PROBE_ORIGIN + "/canary";
|
||||
|
||||
// Requests are parked rather than awaited. An approval prompt only exists
|
||||
// while its call is in flight, so a test that awaited the promise could
|
||||
// never drive the popup that has to settle it; start() files the promise
|
||||
@@ -555,10 +570,9 @@ async function installNetworkStubs(ctx, opts) {
|
||||
// E2E_TRACE_NETWORK=1 prints every request that reaches this handler,
|
||||
// tagged [sw] when it originated in the background service worker.
|
||||
// It exists so the isolation claim above can be re-checked by anyone
|
||||
// in one command, without editing files: the phishing blocklist fetch
|
||||
// showing up with an [sw] tag is the proof that the worker really is
|
||||
// intercepted and that the raw.githubusercontent.com stub below is
|
||||
// live code rather than decoration.
|
||||
// in one command, without editing files: the canary probe and then
|
||||
// every JSON-RPC call behind an approval showing up with an [sw] tag
|
||||
// is the proof that the worker really is intercepted.
|
||||
const trace = traceEnabled(process.env.E2E_TRACE_NETWORK);
|
||||
|
||||
// Regex rather than a glob so chrome-extension:// resource loads are
|
||||
@@ -589,7 +603,11 @@ async function installNetworkStubs(ctx, opts) {
|
||||
// trips run against a real http(s) origin — which is what makes the
|
||||
// shipped content scripts inject at all — without any remote origin
|
||||
// being involved.
|
||||
if (url.origin === DAPP_ORIGIN && p === "/") {
|
||||
if (
|
||||
(url.origin === DAPP_ORIGIN ||
|
||||
url.origin === PHISHING_DAPP_ORIGIN) &&
|
||||
p === "/"
|
||||
) {
|
||||
return route.fulfill({
|
||||
status: 200,
|
||||
contentType: "text/html; charset=utf-8",
|
||||
@@ -635,18 +653,10 @@ async function installNetworkStubs(ctx, opts) {
|
||||
return jsonResponse(route, { Data: {} });
|
||||
}
|
||||
|
||||
// MetaMask phishing blocklist
|
||||
if (
|
||||
url.hostname === "raw.githubusercontent.com" ||
|
||||
p.endsWith("/eth-phishing-detect/main/src/config.json")
|
||||
) {
|
||||
return jsonResponse(route, {
|
||||
version: 2,
|
||||
tolerance: 2,
|
||||
fuzzylist: [],
|
||||
whitelist: [],
|
||||
blacklist: [],
|
||||
});
|
||||
// The interception canary's own request. Answered with nothing: what
|
||||
// is being observed is that it arrived here at all.
|
||||
if (url.href === WORKER_PROBE_URL) {
|
||||
return route.fulfill({ status: 204, body: "" });
|
||||
}
|
||||
|
||||
// Best-effort Etherscan address labels: served as an empty page.
|
||||
@@ -667,12 +677,12 @@ async function installNetworkStubs(ctx, opts) {
|
||||
* Resolve with the first service-worker-originated request this
|
||||
* handler saw, or null if none arrives within `ms`.
|
||||
*
|
||||
* The background worker fetches the phishing blocklist at
|
||||
* startup, unconditionally, within about a second of the context
|
||||
* coming up — so under working interception this resolves almost
|
||||
* immediately. Nothing arriving means worker traffic is bypassing
|
||||
* the handler entirely and going to the real internet, which the
|
||||
* caller turns into a hard failure of the whole suite.
|
||||
* The caller asks the worker for one request of its own (see
|
||||
* WORKER_PROBE_URL) and then waits here, so under working
|
||||
* interception this resolves almost immediately. Nothing arriving
|
||||
* means worker traffic is bypassing the handler entirely and going
|
||||
* to the real internet, which the caller turns into a hard failure
|
||||
* of the whole suite.
|
||||
*/
|
||||
waitForServiceWorkerTraffic(ms) {
|
||||
if (firstWorkerRequest) return Promise.resolve(firstWorkerRequest);
|
||||
@@ -696,6 +706,9 @@ module.exports = {
|
||||
DAPP_HTML,
|
||||
DAPP_ORIGIN,
|
||||
DAPP_URL,
|
||||
PHISHING_DAPP_ORIGIN,
|
||||
PHISHING_DAPP_URL,
|
||||
WORKER_PROBE_URL,
|
||||
FEE_ESTIMATE_WEI,
|
||||
FEE_RESERVE_WEI,
|
||||
STUB_COUNTERPARTY,
|
||||
|
||||
@@ -33,6 +33,7 @@ const {
|
||||
const {
|
||||
DAPP_ORIGIN,
|
||||
DAPP_URL,
|
||||
PHISHING_DAPP_URL,
|
||||
FEE_ESTIMATE_WEI,
|
||||
FEE_RESERVE_WEI,
|
||||
STUB_COUNTERPARTY,
|
||||
@@ -2069,9 +2070,9 @@ async function extensionActiveAddress(page) {
|
||||
return getAddress(address);
|
||||
}
|
||||
|
||||
async function openDapp(ctx) {
|
||||
async function openDapp(ctx, url = DAPP_URL) {
|
||||
const page = await ctx.newPage();
|
||||
await page.goto(DAPP_URL);
|
||||
await page.goto(url);
|
||||
// window.ethereum is not the fixture's doing — it is the shipped
|
||||
// MAIN-world content script. Waiting for it is waiting for the real
|
||||
// provider to have injected itself into a real http(s) origin.
|
||||
@@ -2473,6 +2474,15 @@ test("eth_requestAccounts rejected at the prompt returns a rejection (#183)", as
|
||||
JSON.stringify(hostname),
|
||||
);
|
||||
|
||||
// The control for the phishing test below: this origin is not on the
|
||||
// blocklist, so the banner must be absent here. Without it a banner
|
||||
// that was simply always visible would satisfy that test.
|
||||
assert(
|
||||
await popup.locator("#approve-site-phishing-warning").isHidden(),
|
||||
"the phishing warning is showing for an origin that is not on " +
|
||||
"the blocklist, so its appearance proves nothing",
|
||||
);
|
||||
|
||||
// Deliberately not remembered: a remembered rejection lands the
|
||||
// origin in deniedSites and every later test in this section is
|
||||
// auto-rejected with no prompt at all, which would look like a pass.
|
||||
@@ -2532,6 +2542,53 @@ test("eth_requestAccounts approved returns the selected address (#183)", async (
|
||||
);
|
||||
});
|
||||
|
||||
test("a connect request from a blocklisted site is flagged (#219)", async (env) => {
|
||||
// The vendored blocklist, end to end: a real entry from the shipped
|
||||
// artifact, served as a real http(s) origin, reaching the real background
|
||||
// check and the real approval screen. Nothing about the list is stubbed —
|
||||
// there is nothing left to stub, since the extension no longer fetches it.
|
||||
const phishingDapp = await openDapp(env.ctx, PHISHING_DAPP_URL);
|
||||
const hostname = new URL(PHISHING_DAPP_URL).hostname;
|
||||
try {
|
||||
await reserveApprovalTab(env);
|
||||
await startRequest(
|
||||
phishingDapp,
|
||||
"phishing-accounts",
|
||||
"eth_requestAccounts",
|
||||
[],
|
||||
);
|
||||
const popup = await openSiteApprovalPopup(env);
|
||||
try {
|
||||
await visible(popup, "#view-approve-site");
|
||||
|
||||
const shown = await popup.locator("#approve-hostname").innerText();
|
||||
assert(
|
||||
shown === hostname,
|
||||
"the site prompt names the wrong origin: " +
|
||||
JSON.stringify(shown),
|
||||
);
|
||||
|
||||
await visible(popup, "#approve-site-phishing-warning");
|
||||
console.log("# phishing warning shown for " + hostname);
|
||||
|
||||
// Not remembered: a remembered decision for this origin would
|
||||
// outlive the test.
|
||||
await popup.uncheck("#approve-remember");
|
||||
await popup.click("#btn-reject");
|
||||
|
||||
await assertUserRejection(
|
||||
phishingDapp,
|
||||
"phishing-accounts",
|
||||
"the blocklisted site's eth_requestAccounts",
|
||||
);
|
||||
} finally {
|
||||
await closeApprovalPages(env.ctx);
|
||||
}
|
||||
} finally {
|
||||
await phishingDapp.close();
|
||||
}
|
||||
});
|
||||
|
||||
test("personal_sign signs, and the signature recovers to the address (#183)", async (env) => {
|
||||
await startRequest(env.dapp, "sign", "personal_sign", [
|
||||
SIGN_HEX,
|
||||
|
||||
Reference in New Issue
Block a user