diff --git a/src/lib/__tests__/url-safety.test.ts b/src/lib/__tests__/url-safety.test.ts index d94474fe5..a97c04c03 100644 --- a/src/lib/__tests__/url-safety.test.ts +++ b/src/lib/__tests__/url-safety.test.ts @@ -31,6 +31,23 @@ describe("sanitizeSameOriginPath", () => { expect(sanitizeSameOriginPath("\\\\evil.com", BASE)).toBe("/"); }); + it("rejects a same-origin next whose pathname collapses to protocol-relative", () => { + // `/..//evil.com` resolves ON-origin here (the `..` collapses at root), + // but its pathname is `//evil.com`. Consumers re-resolve the returned + // value (`new URL(result, base)`, `router.push`), where `//evil.com` + // becomes protocol-relative and escapes to https://evil.com. The + // reconstructed-path re-check must reject these to "/". + expect(sanitizeSameOriginPath("/..//evil.com", BASE)).toBe("/"); + expect(sanitizeSameOriginPath("/.//evil.com", BASE)).toBe("/"); + expect(sanitizeSameOriginPath("/../..//evil.com", BASE)).toBe("/"); + }); + + it("still allows a legitimate same-origin path with an inner double slash", () => { + // Only a LEADING `//` (protocol-relative) escapes on re-resolve; a double + // slash inside the path stays on-origin and must be preserved. + expect(sanitizeSameOriginPath("/reports/a//b", BASE)).toBe("/reports/a//b"); + }); + it("preserves a same-origin path's search and hash", () => { expect(sanitizeSameOriginPath("/settings?tab=security#top", BASE)).toBe( "/settings?tab=security#top", diff --git a/src/lib/url-safety.ts b/src/lib/url-safety.ts index b8422e75d..3d0be7bbf 100644 --- a/src/lib/url-safety.ts +++ b/src/lib/url-safety.ts @@ -22,10 +22,19 @@ export function sanitizeSameOriginPath( ): string { if (!next) return "/"; try { - const resolved = new URL(next, baseUrl); const base = new URL(baseUrl); + const resolved = new URL(next, baseUrl); if (resolved.origin !== base.origin) return "/"; - return `${resolved.pathname}${resolved.search}${resolved.hash}`; + const result = `${resolved.pathname}${resolved.search}${resolved.hash}`; + // Consumers commonly RE-RESOLVE the returned value against the origin + // (`new URL(result, base)` in the OIDC callback redirect, `router.push()` + // on the login page). A same-origin `next` can still leave a pathname + // like `//evil.com` — e.g. `/..//evil.com` resolves on-origin here (the + // `..` collapses at root) yet its pathname is protocol-relative, so the + // re-resolve escapes to `https://evil.com`. Re-verify that the + // reconstructed path itself still resolves on-origin; reject otherwise. + if (new URL(result, base).origin !== base.origin) return "/"; + return result; } catch { return "/"; }