17ddf6513cd7
gate the TOTP-selector and confirm-password writes behind the form-action decision (PB-26-105, v5.0.37)
- time
- 2026-09-01 21:23 UTC · 1 month ago
- machine
- koh2
- agent
- hy4-preview
- session
- 2026-09-01 security assessment remediation
- parent
- a5153594b717
↓ peach-browser-17ddf651.tar.gzatlas at this save
files changed8 files
manifest.firefox.json+1 −1manifest.json+1 −1package.json+1 −1src/content/content.dismissal.test.ts+4 −0src/content/content.fill.test.ts+17 −7src/content/content.form-action-gate.test.ts+184 −0src/content/fill.ts+18 −4src/content/index.ts+4 −4manifest.firefox.json+1 −1
@@ -1,7 +1,7 @@11 {22 "manifest_version": 3,33 "name": "Peach Password Manager",4- "version": "5.0.36",4+ "version": "5.0.37",55 "description": "Zero-knowledge password manager",66 "browser_specific_settings": {77 "gecko": {manifest.json+1 −1
@@ -2,7 +2,7 @@22 "manifest_version": 3,33 "minimum_chrome_version": "116",44 "name": "Peach Password Manager",5- "version": "5.0.36",5+ "version": "5.0.37",66 "description": "Zero-knowledge password manager",77 "action": {88 "default_popup": "src/popup/index.html",package.json+1 −1
@@ -1,6 +1,6 @@11 {22 "name": "peach",3- "version": "5.0.36",3+ "version": "5.0.37",44 "description": "Zero-knowledge password manager",55 "license": "GPL-3.0-only",66 "type": "module",src/content/content.dismissal.test.ts+4 −0
@@ -189,6 +189,10 @@189189 // opened with and invoking it — that calls recordFillSuccess internally.190190 async function openAndFill() {191191 const { form, username, password } = createLoginForm();192+ // jsdom's default document URL is http, and PB-26-008 refuses to fill any193+ // form whose action is not https on the page domain — an unactioned form194+ // inherits the document URL, so the fixture must declare one.195+ form.action = "https://localhost/login";192196 withEntries();193197 await handleFieldInteraction(password);194198 await vi.waitFor(() => expect(showSelector).toHaveBeenCalled());src/content/content.fill.test.ts+17 −7
@@ -181,18 +181,28 @@181181 expect(other.value).toBe("do-not-touch");182182 });183183 184- // T-FL12: the brief assumes fillFields returns the filled set; the real API185- // returns void and the caller (index.ts recordFillSuccess) tracks success by186- // the elements it passed. We pin the actual contract: fillFields returns187- // undefined, and an invisible password is intentionally skipped — which is how188- // a caller can tell a fill was partial.189- it("T-FL12: fillFields returns void and skips an invisible password field", () => {184+ // T-FL12: fillFields reports whether the form-action gate allowed the fill185+ // (PB-26-105) — callers use it to decide whether any further field writes or186+ // side effects may follow. An invisible password is intentionally skipped,187+ // which is still not reported as a refusal.188+ it("T-FL12: fillFields reports gate permission and skips an invisible password field", () => {190189 const u = makeInput({ type: "text" });191190 const p = makeInput({ type: "password" });192191 p.style.display = "none"; // invisible → isVisible() false → not filled193192 const result = fillFields(u, p, "alice", "s3cret");194- expect(result).toBeUndefined();193+ expect(result).toBe(true);195194 expect(u.value).toBe("alice");196195 expect(p.value).toBe(""); // skipped because not visible196+ });197+198+ it("T-FL12b: fillFields reports false when the form-action gate refuses", () => {199+ const form = document.createElement("form");200+ form.action = "https://evil.example/login";201+ document.body.appendChild(form);202+ const u = makeInput({ type: "text" }, form);203+ const p = makeInput({ type: "password" }, form);204+ expect(fillFields(u, p, "alice", "s3cret", form, "localhost")).toBe(false);205+ expect(u.value).toBe("");206+ expect(p.value).toBe("");197207 });198208 });src/content/content.form-action-gate.test.ts+184 −0
@@ -0,0 +1,184 @@1+// @vitest-environment jsdom2+//3+// PB-26-105 — the form-action phishing gate must be enforced on every4+// content-script fill path, not only fillFields. Covers the three paths the5+// finding cites: the TOTP-only selector, the signup "copy & fill" surface,6+// and the credential path's TOTP/clipboard/canary side effects.7+8+import { vi } from "vitest";9+10+const { predictBatch, dismissSpy } = vi.hoisted(() => ({11+ predictBatch: vi.fn(() => Promise.resolve([] as unknown[])),12+ dismissSpy: vi.fn(),13+}));14+15+vi.mock("./inject", () => ({16+ injectFieldIcon: vi.fn(() => vi.fn()),17+ showAutofillSelector: vi.fn(() => dismissSpy),18+ showTOTPSelector: vi.fn(() => dismissSpy),19+ showCardSelector: vi.fn(() => dismissSpy),20+ showRememberSiteToast: vi.fn(),21+ showPasskeyAvailable: vi.fn(),22+ showPasskeyProGate: vi.fn(),23+ showPasskeyRegistrationOffer: vi.fn(),24+}));25+26+vi.mock("./content-ml-detector", () => ({27+ contentMLDetector: { predictBatch },28+}));29+30+import { afterEach, beforeEach, describe, expect, it } from "vitest";31+32+// Each test reloads the content module: index.ts keeps page-wide fill state33+// (PAGE_FILL_GRACE_MS) that would otherwise suppress the selector in every34+// test after the first successful fill.35+async function loadSubject() {36+ vi.resetModules();37+ const setup = await import("./_setup");38+ const index = await import("./index");39+ const inject = await import("./inject");40+ return {41+ setup,42+ handleFieldInteraction: index.handleFieldInteraction,43+ showSelector: inject.showAutofillSelector as unknown as ReturnType<typeof vi.fn>,44+ showTOTP: inject.showTOTPSelector as unknown as ReturnType<typeof vi.fn>,45+ };46+}47+48+beforeEach(() => {49+ document.body.innerHTML = "";50+ predictBatch.mockReset();51+ predictBatch.mockReturnValue(Promise.resolve([]));52+});53+54+afterEach(async () => {55+ vi.useRealTimers();56+ // index.ts's url-watcher MutationObserver reads location.href; flush it while57+ // jsdom globals still exist.58+ await new Promise((r) => setTimeout(r, 0));59+});60+61+describe("PB-26-105: form-action gate on every fill path", () => {62+ it("TOTP-only path refuses a live code when the form posts off-site", async () => {63+ const { setup, handleFieldInteraction, showTOTP } = await loadSubject();64+ const { makeInput } = setup;65+ const form = document.createElement("form");66+ form.action = "https://evil.example/verify";67+ document.body.appendChild(form);68+ const otp = makeInput(69+ { type: "text", name: "otp", autocomplete: "one-time-code" },70+ form,71+ );72+ setup.setMessageResponder((msg: any) => {73+ if (msg.type === "AUTOFILL_QUERY") {74+ return { ok: true, data: [{ id: "t1", title: "Acme", username: "alice" }] };75+ }76+ if (msg.type === "GET_ENTRY_FOR_FILL") {77+ return {78+ ok: true,79+ data: { id: "t1", title: "Acme", username: "alice", totpCode: "123456" },80+ };81+ }82+ return { ok: true, data: null };83+ });84+85+ await handleFieldInteraction(otp, true);86+ await vi.waitFor(() => expect(showTOTP).toHaveBeenCalled());87+ await (showTOTP.mock.calls.at(-1)![2] as (id: string) => Promise<void>)("t1");88+89+ expect(otp.value).toBe("");90+ });91+92+ it("TOTP-only path still fills when the form posts to the page's own domain", async () => {93+ const { setup, handleFieldInteraction, showTOTP } = await loadSubject();94+ const form = document.createElement("form");95+ form.action = "https://localhost/verify";96+ document.body.appendChild(form);97+ const otp = setup.makeInput(98+ { type: "text", name: "otp", autocomplete: "one-time-code" },99+ form,100+ );101+ setup.setMessageResponder((msg: any) => {102+ if (msg.type === "AUTOFILL_QUERY") {103+ return { ok: true, data: [{ id: "t1", title: "Acme", username: "alice" }] };104+ }105+ if (msg.type === "GET_ENTRY_FOR_FILL") {106+ return {107+ ok: true,108+ data: { id: "t1", title: "Acme", username: "alice", totpCode: "123456" },109+ };110+ }111+ return { ok: true, data: null };112+ });113+114+ await handleFieldInteraction(otp, true);115+ await vi.waitFor(() => expect(showTOTP).toHaveBeenCalled());116+ await (showTOTP.mock.calls.at(-1)![2] as (id: string) => Promise<void>)("t1");117+118+ expect(otp.value).toBe("123456");119+ });120+121+ it("signup fill writes neither password nor confirm-password when refused", async () => {122+ const { setup, handleFieldInteraction, showSelector } = await loadSubject();123+ const form = document.createElement("form");124+ form.action = "https://evil.example/signup";125+ document.body.appendChild(form);126+ setup.makeInput({ type: "text", name: "username", autocomplete: "username" }, form);127+ const password = setup.makeInput(128+ { type: "password", name: "password", autocomplete: "new-password" },129+ form,130+ );131+ const confirm = setup.makeInput(132+ { type: "password", name: "confirmPassword", autocomplete: "new-password" },133+ form,134+ );135+ setup.setMessageResponder((msg: any) =>136+ msg.type === "AUTOFILL_QUERY" ? { ok: true, data: [] } : { ok: true, data: null },137+ );138+139+ await handleFieldInteraction(password, true);140+ await vi.waitFor(() => expect(showSelector).toHaveBeenCalled());141+142+ const options = showSelector.mock.calls.at(-1)![3] as any;143+ expect(options.isSignup).toBe(true);144+ await options.onFillFields("alice", "correct-horse-battery", "");145+146+ expect(password.value).toBe("");147+ expect(confirm.value).toBe("");148+ });149+150+ it("refused credential fill writes no TOTP, no clipboard and no canary ping", async () => {151+ const { setup, handleFieldInteraction, showSelector } = await loadSubject();152+ const { form, username, password } = setup.createLoginForm();153+ form.action = "https://evil.example/login";154+ setup.setMessageResponder((msg: any) => {155+ if (msg.type === "AUTOFILL_QUERY") {156+ return { ok: true, data: [{ id: "e1", title: "Acme", username: "alice" }] };157+ }158+ if (msg.type === "GET_ENTRY_FOR_FILL") {159+ return {160+ ok: true,161+ data: {162+ id: "e1",163+ title: "Acme",164+ username: "alice",165+ password: "correct-horse",166+ totpCode: "123456",167+ canary: { webhookUrl: "https://hooks.example/x", pingCount: 0 },168+ },169+ };170+ }171+ return { ok: true, data: null };172+ });173+174+ await handleFieldInteraction(username, true);175+ await vi.waitFor(() => expect(showSelector).toHaveBeenCalled());176+177+ await (showSelector.mock.calls.at(-1)![2] as (id: string) => Promise<void>)("e1");178+179+ expect(username.value).toBe("");180+ expect(password.value).toBe("");181+ expect(setup.clipboardWrite).not.toHaveBeenCalled();182+ expect(setup.sentMessages.some((m: any) => m?.type === "FIRE_CANARY_PING")).toBe(false);183+ });184+});src/content/fill.ts+18 −4
@@ -22,6 +22,19 @@2222 return true;2323 }2424 25+// Single decision for every fill path: PB-26-105 was three copies of this26+// condition with one of them missing, so a gate refusal in fillFields still27+// let the TOTP and confirm-password paths write into the hijacked form.28+export function fillAllowed(29+ form: HTMLFormElement | null | undefined,30+ expectedDomain?: string | null31+): boolean {32+ if (!form || !expectedDomain) return true;33+ return checkFormAction(form, expectedDomain);34+}35+36+// Returns false when the form-action gate refused — callers must not write37+// anything else into the form after a refusal.2538 export function fillFields(2639 usernameEl: HTMLInputElement | null,2740 passwordEl: HTMLInputElement | null,@@ -31,9 +44,9 @@3144 expectedDomain?: string,3245 emailEl?: HTMLInputElement | null,3346 email?: string34-): void {35- if (expectedDomain && form && !checkFormAction(form, expectedDomain)) {36- return;47+): boolean {48+ if (!fillAllowed(form, expectedDomain)) {49+ return false;3750 }3851 // Fill username regardless of visibility — on SPA step-by-step login flows3952 // the username input may be visually hidden but still tracked by the form.@@ -46,6 +59,7 @@4659 if (emailEl && email && isVisible(emailEl)) {4760 setNativeValue(emailEl, email);4861 }62+ return true;4963 }5064 5165 // Distributes a TOTP code character-by-character into an ordered array of@@ -67,7 +81,7 @@6781 ): boolean {6882 if (!totpEl || !code || !isVisible(totpEl)) return false;6983 if (totpEl.value.trim() !== "") return false;70- if (expectedDomain && form && !checkFormAction(form, expectedDomain)) return false;84+ if (!fillAllowed(form, expectedDomain)) return false;7185 setNativeValue(totpEl, code);7286 return true;7387 }src/content/index.ts+4 −4
@@ -27,7 +27,7 @@2727 } from "./inject";2828 import { detectBrowser } from "../shared/browser";2929 import { FIREFOX_USE_CONTENT_SCRIPT_PROXY } from "../shared/passkey-flags";30-import { fillCardForm, fillFields, fillOTPDigitBoxes, fillTOTPCode, setNativeValue, checkFormAction } from "./fill";30+import { fillAllowed, fillCardForm, fillFields, fillOTPDigitBoxes, fillTOTPCode, setNativeValue } from "./fill";3131 import { resolveAutofillInteractionTargets } from "./interaction";3232 import { setupSaveDetection } from "./save";3333 import { contentMLDetector } from "./content-ml-detector";@@ -322,8 +322,7 @@322322 // it looks like an email address (sites that store email-as-username and have a323323 // separate email field on signup forms).324324 const emailValue = entry.email ?? (entry.username?.includes("@") ? entry.username : "");325- fillFields(usernameEl, passwordEl, entry.username ?? "", entry.password ?? "", form, pageDomain, emailEl, emailValue);326- const gatePassed = !form || !pageDomain || checkFormAction(form, pageDomain);325+ const gatePassed = fillFields(usernameEl, passwordEl, entry.username ?? "", entry.password ?? "", form, pageDomain, emailEl, emailValue);327326 let filledTOTPField: HTMLInputElement | null = null;328327 329328 if (entry.canary && gatePassed) {@@ -511,7 +510,7 @@511510 savedIdentities,512511 onFillFields: (username, password, email) => {513512 const form = (passwordEl ?? usernameEl)?.closest("form") as HTMLFormElement | null ?? null;514- fillFields(usernameEl, passwordEl, username, password, form, getPageDomain(), emailEl, email);513+ if (!fillFields(usernameEl, passwordEl, username, password, form, getPageDomain(), emailEl, email)) return;515514 if (password && confirmEl) setNativeValue(confirmEl, password);516515 recordFillSuccess(anchorField, usernameEl, passwordEl);517516 },@@ -568,6 +567,7 @@568567 const entry = entryResponse?.ok ? entryResponse.data : undefined;569568 const code = (entry as { totpCode?: string } | undefined)?.totpCode;570569 if (!code) return;570+ if (!fillAllowed(anchorField.closest("form"), getPageDomain())) return;571571 if (otpDigitBoxes && otpDigitBoxes.length > 1) {572572 fillOTPDigitBoxes(otpDigitBoxes, code);573573 } else {