diff --git a/web/src/lib/auth.svelte.ts b/web/src/lib/auth.svelte.ts index 6ef93647c..b94661259 100644 --- a/web/src/lib/auth.svelte.ts +++ b/web/src/lib/auth.svelte.ts @@ -33,6 +33,7 @@ import { upsertAccount } from "./auth/accounts"; import { writePendingLogin } from "./auth/pending"; +import { safeReturnTo } from "./auth/returnTo"; export const AUTH_KEY = Symbol("auth"); const DEV_REDIRECT_URI = "http://127.0.0.1:5173/oauth/callback"; @@ -185,8 +186,7 @@ const resolveProfile = async ( const returnToFromState = (state: object | null): string => { if (state && typeof state === "object" && "returnTo" in state) { - const returnTo = state.returnTo; - if (typeof returnTo === "string") return returnTo; + return safeReturnTo(state.returnTo); } return "/"; }; @@ -355,10 +355,11 @@ export const createAuth = ( resetLoggedOut(lastFailure); }; - const signIn = async (identifier: string, returnTo = "/") => { + const signIn = async (identifier: string, rawReturnTo = "/") => { if (!browser) return; configure(); + const returnTo = safeReturnTo(rawReturnTo); const trimmed = identifier.trim(); if (!trimmed) { state = failureState("Handle or DID required"); diff --git a/web/src/lib/auth/pending.ts b/web/src/lib/auth/pending.ts index 36b50c7d8..7601fe641 100644 --- a/web/src/lib/auth/pending.ts +++ b/web/src/lib/auth/pending.ts @@ -1,5 +1,6 @@ import { browser } from "$app/environment"; import type { Did } from "@atcute/lexicons/syntax"; +import { safeReturnTo } from "./returnTo"; const PENDING_LOGIN_KEY = "tangled.pendingLogin"; const PENDING_CALLBACK_KEY = "tangled.pendingCallback"; @@ -39,7 +40,7 @@ export const readPendingLogin = (): PendingLogin | null => { const parsed: unknown = JSON.parse(raw); if (!isPendingLogin(parsed)) return null; if (Date.now() - parsed.at > PENDING_LOGIN_MAX_AGE_MS) return null; - return parsed; + return { ...parsed, returnTo: safeReturnTo(parsed.returnTo) }; } catch { return null; } @@ -70,7 +71,11 @@ export const consumePendingCallback = (): PendingCallback | null => { ) { return null; } - return parsed as PendingCallback; + const callback = parsed as PendingCallback; + return { + ...callback, + login: { ...callback.login, returnTo: safeReturnTo(callback.login.returnTo) } + }; } catch { return null; } diff --git a/web/src/lib/auth/returnTo.test.ts b/web/src/lib/auth/returnTo.test.ts new file mode 100644 index 000000000..4261f2979 --- /dev/null +++ b/web/src/lib/auth/returnTo.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { safeReturnTo } from "./returnTo"; + +describe("safeReturnTo", () => { + it("keeps same-origin paths with query and hash", () => { + expect(safeReturnTo("/oppi.li/core?tab=pulls#c1")).toBe("/oppi.li/core?tab=pulls#c1"); + }); + + it("rejects network-path references", () => { + expect(safeReturnTo("//evil.example")).toBe("/"); + expect(safeReturnTo("/\\evil.example")).toBe("/"); + expect(safeReturnTo("\\\\evil.example")).toBe("/"); + }); + + it("rejects absolute and non-http targets", () => { + expect(safeReturnTo("https://evil.example/repo")).toBe("/"); + expect(safeReturnTo("javascript:alert(1)")).toBe("/"); + }); + + it("normalizes scheme-relative input that stays on the origin", () => { + // the URL parser reads this as a same-origin path, not a host + expect(safeReturnTo("https:/evil.example")).toBe("/evil.example"); + }); + + it("falls back for empty or non-string input", () => { + expect(safeReturnTo("")).toBe("/"); + expect(safeReturnTo(" ")).toBe("/"); + expect(safeReturnTo(undefined)).toBe("/"); + expect(safeReturnTo(42)).toBe("/"); + }); +}); diff --git a/web/src/lib/auth/returnTo.ts b/web/src/lib/auth/returnTo.ts new file mode 100644 index 000000000..a7cf4588a --- /dev/null +++ b/web/src/lib/auth/returnTo.ts @@ -0,0 +1,19 @@ +// post-login targets are attacker-controlled (login links, stored pending +// state), so resolve against an opaque origin and only accept paths that +// stay on it. also covers network-path forms like "/\evil.com" that the url +// parser turns into a foreign host +const SAFE_BASE = "https://return-to.invalid"; + +export const safeReturnTo = (value: unknown): string => { + if (typeof value !== "string" || value.trim() === "") return "/"; + + let resolved: URL; + try { + resolved = new URL(value, SAFE_BASE); + } catch { + return "/"; + } + if (resolved.origin !== SAFE_BASE) return "/"; + + return `${resolved.pathname}${resolved.search}${resolved.hash}`; +};