From 5ff9e1dd5d3f72cd4c69cc1a0bb0e6e1e134f189 Mon Sep 17 00:00:00 2001 From: cnYui Date: Sun, 6 Sep 2026 08:55:15 +0900 Subject: [PATCH 1/2] fix(config): ignore non relative return paths on the maintenance redirect Both maintenance middlewares handed the user controlled `return` query param straight to NextResponse.redirect, so /maintenance?return= issued a 307 to an arbitrary origin. Resolve the value against the request URL and fall back to "/" unless it stays on the same origin. Fixes #2564 Co-Authored-By: Claude Opus 4.8 --- apps/deploy-web/src/middleware.spec.ts | 24 +++++++++++++++++ apps/deploy-web/src/middleware.ts | 5 +++- apps/stats-web/src/middleware.spec.ts | 36 ++++++++++++++++++++++++++ apps/stats-web/src/middleware.ts | 5 +++- 4 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 apps/stats-web/src/middleware.spec.ts diff --git a/apps/deploy-web/src/middleware.spec.ts b/apps/deploy-web/src/middleware.spec.ts index 7d08e5a664..6d78d1d777 100644 --- a/apps/deploy-web/src/middleware.spec.ts +++ b/apps/deploy-web/src/middleware.spec.ts @@ -37,6 +37,30 @@ describe("middleware", () => { expect(response.headers.get("Content-Security-Policy-Report-Only")).toBeNull(); }); + it("redirects to the requested relative path when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2Fdeployments%3Ftab%3Dactive" }); + + expect(response.headers.get("location")).toBe("http://localhost/deployments?tab=active"); + }); + + it("ignores an absolute return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=https%3A%2F%2Fevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + + it("ignores a protocol relative return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2F%2Fevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + + it("ignores a backslash prefixed return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2F%5Cevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + function setup(input: { path: string }) { const request = new NextRequest(new URL(`http://localhost${input.path}`)); const response = middleware(request); diff --git a/apps/deploy-web/src/middleware.ts b/apps/deploy-web/src/middleware.ts index 5e2b81ed3a..f2d50b7028 100644 --- a/apps/deploy-web/src/middleware.ts +++ b/apps/deploy-web/src/middleware.ts @@ -80,8 +80,11 @@ function getReturnPath(request: NextRequest) { try { const returnParam = request.nextUrl.searchParams.get("return"); const returnPath = returnParam ? decodeURIComponent(returnParam) : "/"; + const requestUrl = new URL(request.url); + const returnUrl = new URL(returnPath, requestUrl); + const isSameOrigin = returnUrl.origin === requestUrl.origin; - return returnPath; + return isSameOrigin ? `${returnUrl.pathname}${returnUrl.search}${returnUrl.hash}` : "/"; } catch (error) { logger.error({ message: "Failed to get return path", error }); return "/"; diff --git a/apps/stats-web/src/middleware.spec.ts b/apps/stats-web/src/middleware.spec.ts new file mode 100644 index 0000000000..002c4dbdfa --- /dev/null +++ b/apps/stats-web/src/middleware.spec.ts @@ -0,0 +1,36 @@ +import { NextRequest } from "next/server"; +import { describe, expect, it } from "vitest"; + +import { middleware } from "./middleware"; + +describe("middleware", () => { + it("redirects to the requested relative path when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2Fgraph%2Fdaily-akt-spent" }); + + expect(response.headers.get("location")).toBe("http://localhost/graph/daily-akt-spent"); + }); + + it("ignores an absolute return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=https%3A%2F%2Fevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + + it("ignores a protocol relative return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2F%2Fevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + + it("ignores a backslash prefixed return url when leaving the maintenance page", () => { + const { response } = setup({ path: "/maintenance?return=%2F%5Cevil.example%2Fphish" }); + + expect(response.headers.get("location")).toBe("http://localhost/"); + }); + + function setup(input: { path: string }) { + const request = new NextRequest(new URL(`http://localhost${input.path}`)); + const response = middleware(request); + return { request, response }; + } +}); diff --git a/apps/stats-web/src/middleware.ts b/apps/stats-web/src/middleware.ts index 35a6759c93..4a64668095 100644 --- a/apps/stats-web/src/middleware.ts +++ b/apps/stats-web/src/middleware.ts @@ -64,8 +64,11 @@ function getReturnPath(request: NextRequest) { try { const returnParam = request.nextUrl.searchParams.get("return"); const returnPath = returnParam ? decodeURIComponent(returnParam) : "/"; + const requestUrl = new URL(request.url); + const returnUrl = new URL(returnPath, requestUrl); + const isSameOrigin = returnUrl.origin === requestUrl.origin; - return returnPath; + return isSameOrigin ? `${returnUrl.pathname}${returnUrl.search}${returnUrl.hash}` : "/"; } catch (error) { logger.error({ message: "Failed to get return path", error }); return "/"; From 52ec89a54724b64763bca3d7610f5647b82cee7f Mon Sep 17 00:00:00 2001 From: cnYui Date: Fri, 11 Sep 2026 21:32:54 +0900 Subject: [PATCH 2/2] fix(config): prevent open redirect via same-origin protocol-relative return path A return param like `http://host//evil.example/phish` passes the origin check but its pathname is `//evil.example/phish`; returning that string and re-parsing it against the request URL treats it as a protocol-relative URL and redirects to an external origin (CWE-601). Return the validated same-origin URL object and pass it directly to NextResponse.redirect, so the pathname is never re-parsed. Adds a regression test. Co-Authored-By: Claude Opus 4.8 --- apps/deploy-web/src/middleware.spec.ts | 8 ++++++++ apps/deploy-web/src/middleware.ts | 16 ++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/apps/deploy-web/src/middleware.spec.ts b/apps/deploy-web/src/middleware.spec.ts index a3eb4ce59a..53d57a4a9a 100644 --- a/apps/deploy-web/src/middleware.spec.ts +++ b/apps/deploy-web/src/middleware.spec.ts @@ -108,6 +108,14 @@ describe("middleware", () => { expect(response.headers.get("location")).toBe("http://localhost/"); }); + it("keeps a same-origin absolute return url with a protocol-relative path on the request origin", () => { + const { response } = setup({ path: "/maintenance?return=http%3A%2F%2Flocalhost%2F%2Fevil.example%2Fphish" }); + + const location = new URL(response.headers.get("location") ?? ""); + expect(location.host).toBe("localhost"); + expect(location.hostname).not.toBe("evil.example"); + }); + function setup(input: { path: string }) { const request = new NextRequest(new URL(`http://localhost${input.path}`)); const response = middleware(request); diff --git a/apps/deploy-web/src/middleware.ts b/apps/deploy-web/src/middleware.ts index 51f34cc28b..ec1a12990c 100644 --- a/apps/deploy-web/src/middleware.ts +++ b/apps/deploy-web/src/middleware.ts @@ -41,10 +41,10 @@ export function middleware(request: NextRequest) { setContentSecurityPolicyHeaders(redirectResponse, contentSecurityPolicyHeaderName, contentSecurityPolicy, contentSecurityPolicyReportHeaders); return redirectResponse; } else if (!isMaintenanceMode && pathname.startsWith(maintenancePage)) { - const returnPath = getReturnPath(request); - logger.info({ message: `Redirecting from maintenance page to ${returnPath}` }); + const returnUrl = getReturnPath(request); + logger.info({ message: `Redirecting from maintenance page to ${returnUrl.pathname}${returnUrl.search}` }); - const redirectResponse = NextResponse.redirect(new URL(returnPath, request.url), 307); // 307 - temporary redirect + const redirectResponse = NextResponse.redirect(returnUrl, 307); // 307 - temporary redirect setContentSecurityPolicyHeaders(redirectResponse, contentSecurityPolicyHeaderName, contentSecurityPolicy, contentSecurityPolicyReportHeaders); return redirectResponse; } @@ -82,17 +82,21 @@ function setContentSecurityPolicyHeaders( } function getReturnPath(request: NextRequest) { + const requestUrl = new URL(request.url); try { const returnParam = request.nextUrl.searchParams.get("return"); const returnPath = returnParam ? decodeURIComponent(returnParam) : "/"; - const requestUrl = new URL(request.url); const returnUrl = new URL(returnPath, requestUrl); const isSameOrigin = returnUrl.origin === requestUrl.origin; - return isSameOrigin ? `${returnUrl.pathname}${returnUrl.search}${returnUrl.hash}` : "/"; + // Return the validated same-origin URL object rather than a pathname string. + // A same-origin absolute return (e.g. `http://host//evil.example/phish`) yields a + // pathname of `//evil.example/phish`; re-parsing that string against the request URL + // treats it as a protocol-relative URL and escapes to an external origin (CWE-601). + return isSameOrigin ? returnUrl : new URL("/", requestUrl); } catch (error) { logger.error({ message: "Failed to get return path", error }); - return "/"; + return new URL("/", requestUrl); } }