mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
fix(security): reject a same-origin next path that re-resolves protocol-relative
sanitizeSameOriginPath returned pathname+search+hash, so a crafted relative next like /..//evil.com passed the origin check but left a //evil.com pathname that re-resolves to a foreign origin when a consumer re-feeds it to new URL(result, base) or router.push. Re-verify the reconstructed path resolves on-origin; reject to / otherwise. Covers the OIDC callback and the password/passkey login redirect.
This commit is contained in:
1 parent
60658be776
commit
d8e48a6304
2 files changed
+28
-2
No files matched your search
@@ -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",
|
||||
|
||||
+11
-2
@@ -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 "/";
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user