diff --git a/telemetry-worker/src/auth.ts b/telemetry-worker/src/auth.ts index b2f10562..3d913616 100644 --- a/telemetry-worker/src/auth.ts +++ b/telemetry-worker/src/auth.ts @@ -265,9 +265,14 @@ export function cleanEmail(v: unknown): string | null { } /** Only same-site paths under /account (a team invite link, /account/join/, is one) or /login/device may be a `next` target. */ +/** + * Where a sign-in may send the browser afterwards (?next=): our own account pages, the device page the genclass command + * opens, and the page check (/scan, or /scan?url=, so "Sign in to see it all" on a report comes + * back to that report). Anything else is null (the caller goes to /account). + */ export function safeNext(v: string | null | undefined): string | null { if (!v || v.length > 300 || !v.startsWith("/") || v.startsWith("//") || /[\\\u0000-\u001f]/.test(v)) return null; - return /^\/(account(\/[A-Za-z0-9/]*)?([?#].*)?|login\/device(\?code=[A-Za-z-]{0,12})?)$/.test(v) ? v : null; + return /^\/(account(\/[A-Za-z0-9/]*)?([?#].*)?|login\/device(\?code=[A-Za-z-]{0,12})?|scan(\?url=[A-Za-z0-9%._~!*'()\/:+-]{1,280})?)$/.test(v) ? v : null; } export function fail(status: number, error: string, headers: Record = {}): Response { diff --git a/telemetry-worker/test/auth.test.ts b/telemetry-worker/test/auth.test.ts index 542bb8e5..6fd23810 100644 --- a/telemetry-worker/test/auth.test.ts +++ b/telemetry-worker/test/auth.test.ts @@ -1379,9 +1379,12 @@ describe("accounts: CBOR, signatures, links (unit)", () => { expect(() => derToRaw(Uint8Array.from([0x31, 0x00]))).toThrow(); }); - it("next targets stay on our own account pages; dashboard links parse", () => { - for (const ok of ["/account", "/account/apps/0123456789abcdef", "/login/device?code=BCDF-GHJK", "/account?welcome=1"]) expect(safeNext(ok)).toBe(ok); - for (const bad of ["//evil.example", "/\\evil.example", "https://evil.example", "/start", "/account/../x", "/login/device?code=", null]) expect(safeNext(bad as string)).toBeNull(); + it("next targets stay on our own account pages and the page check; dashboard links parse", () => { + for (const ok of ["/account", "/account/apps/0123456789abcdef", "/login/device?code=BCDF-GHJK", "/account?welcome=1", + "/scan", "/scan?url=https%3A%2F%2Fshop.example.com%2Fpricing%3Fa%3D1", "/scan?url=stripe.com/pricing"]) expect(safeNext(ok)).toBe(ok); + for (const bad of ["//evil.example", "/\\evil.example", "https://evil.example", "/start", "/account/../x", "/login/device?code=", null, + "/scanner", "/scan/../account", "/scan?url=", "/scan?url=", '/scan?url=a"b', "/scan?url=a&next=//evil.example", "/scan?url=a#x", "/scan?other=1", + "/scan?url=" + "a".repeat(281)]) expect(safeNext(bad as string)).toBeNull(); const s = "A".repeat(32); expect(secretFromLink(`https://genclass.dev/dashboard/${s}`)).toBe(s); expect(secretFromLink(` /dashboard/${s} `)).toBe(s);