Denial of Service (DoS) via server action forwarding loop in middleware-forwarded requests

PoC reproduction (pre-fix behavior): Setup a Next.js app using app-dir with a middleware that rewrites server action POSTs from /with-action to /without-action, where /without-action does not bundle the action. The forward path will bounce the request between workers that host/bundle the action and those that don’t, potentially looping until network timeouts or memory pressure. Files to add (example): 1) middleware.ts import { NextResponse } from 'next/server' import type { NextRequest } from 'next/server' export function middleware(request: NextRequest) { // Only rewrite POSTs to the action route to a route that does not bundle it if (request.method === 'POST' && request.nextUrl.pathname === '/with-action') { const url = request.nextUrl.clone() url.pathname = '/without-action' return NextResponse.rewrite(url) } return NextResponse.next() } export const config = { matcher: '/with-action', } 2) app-dir/pages (or app-router) setup: - /with-action/page.tsx with a server action attached to a form (use server) wrapped in an error boundary that looks for an UnrecognizedActionError - /without-action/page.tsx a page that does not bundle the action (no server action) Example snippet for with-action (server action on form): 'use client' export default function Page() { return ( <form action={async () => { 'use server' }}> <button id="run-action">Run action</button> </form> ) } 3) Client-side test (optional): - Navigate to /with-action and click the Run action button - Observe repeated forwarding attempts between /with-action and /without-action as the middleware rewrites the POST, causing a loop if the guard is not present Expected post-fix behavior (what you should observe after applying this patch): - The server detects x-action-forwarded (via header) and does not forward again, preventing the loop - If the forwarded worker cannot find the action, the response includes x-nextjs-action-not-found header and 404 with a clear UnrecognizedActionError on the client, avoiding a generic error Notes: - The test in the commit (test/e2e/app-dir/action-forward-loop) reproduces the scenario and asserts the presence of the action-not-found error when the loop is broken by the fix. - The core exploit payload here is the forwarding loop induced by middleware rewrites in the absence of the new guard; the PoC above demonstrates how a forward loop is triggered and why the guard is necessary.

diff --git a/packages/next/src/server/app-render/action-handler.ts b/packages/next/src/server/app-render/action-handler.ts index 5048ddba614..a5d75316b2f 100644 --- a/packages/next/src/server/app-render/action-handler.ts +++ b/packages/next/src/server/app-render/action-handler.ts @@ -279,9 +279,18 @@ async function createForwardedActionResponse( } return new FlightRenderResult(response.body!) - } else { - // Since we aren't consuming the response body, we cancel it to avoid memory leaks - response.body?.cancel() + } + + // Since we aren't consuming the response body, we cancel it to avoid memory leaks + response.body?.cancel() + + // Pass the action-not-found marker through so the client throws + // UnrecognizedActionError instead of a generic "unexpected response". + if (response.headers.get(NEXT_ACTION_NOT_FOUND_HEADER) === '1') { + res.setHeader(NEXT_ACTION_NOT_FOUND_HEADER, '1') + res.setHeader('content-type', 'text/plain') + res.statusCode = 404 + return RenderResult.fromStatic('Server action not found.', 'text/plain') } } catch (err) { // we couldn't stream the forwarded response, so we'll just return an empty response @@ -706,11 +715,15 @@ export async function handleAction({ const actionWasForwarded = Boolean(req.headers['x-action-forwarded']) - if (actionId) { + // Only attempt to forward if this request has not already been forwarded. + // Otherwise middleware that rewrites the action POST can cause the receiving + // worker to forward again, looping indefinitely. + if (actionId && !actionWasForwarded) { const forwardedWorker = selectWorkerForForwarding(actionId, page) - // If forwardedWorker is truthy, it means there isn't a worker for the action - // in the current handler, so we forward the request to a worker that has the action. + // If forwardedWorker is truthy, it means there isn't a worker for the + // action in the current handler, so we forward the request to a worker that + // has the action. if (forwardedWorker) { return { type: 'done', diff --git a/test/e2e/app-dir/action-forward-loop/action-forward-loop.test.ts b/test/e2e/app-dir/action-forward-loop/action-forward-loop.test.ts new file mode 100644 index 00000000000..7d90d696e38 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/action-forward-loop.test.ts @@ -0,0 +1,13 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('action forward loop prevention', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('renders the action-not-found error when a rewrite sends the action POST to a route that does not bundle it', async () => { + const browser = await next.browser('/with-action') + await browser.elementById('run-action').click() + await browser.waitForElementByCss('#action-not-found-error') + }) +}) diff --git a/test/e2e/app-dir/action-forward-loop/app/layout.tsx b/test/e2e/app-dir/action-forward-loop/app/layout.tsx new file mode 100644 index 00000000000..888614deda3 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/app/layout.tsx @@ -0,0 +1,8 @@ +import { ReactNode } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + <html> + <body>{children}</body> + </html> + ) +} diff --git a/test/e2e/app-dir/action-forward-loop/app/with-action/error-boundary.tsx b/test/e2e/app-dir/action-forward-loop/app/with-action/error-boundary.tsx new file mode 100644 index 00000000000..b581b6d1e54 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/app/with-action/error-boundary.tsx @@ -0,0 +1,27 @@ +'use client' + +import { Component, type ReactNode } from 'react' +import { unstable_isUnrecognizedActionError } from 'next/navigation' + +interface State { + error: unknown +} + +export class ErrorBoundary extends Component<{ children: ReactNode }, State> { + state: State = { error: null } + + static getDerivedStateFromError(error: unknown): State { + return { error } + } + + render() { + const { error } = this.state + if (error === null) { + return this.props.children + } + if (unstable_isUnrecognizedActionError(error)) { + return <p id="action-not-found-error">Server action not found</p> + } + return <p id="unexpected-error">Unexpected error</p> + } +} diff --git a/test/e2e/app-dir/action-forward-loop/app/with-action/page.tsx b/test/e2e/app-dir/action-forward-loop/app/with-action/page.tsx new file mode 100644 index 00000000000..02195f78690 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/app/with-action/page.tsx @@ -0,0 +1,18 @@ +import { ErrorBoundary } from './error-boundary' + +export default function Page() { + return ( + <main> + <h1 id="with-action-page">with-action</h1> + <ErrorBoundary> + <form + action={async () => { + 'use server' + }} + > + <button id="run-action">Run action</button> + </form> + </ErrorBoundary> + </main> + ) +} diff --git a/test/e2e/app-dir/action-forward-loop/app/without-action/page.tsx b/test/e2e/app-dir/action-forward-loop/app/without-action/page.tsx new file mode 100644 index 00000000000..d34252e0929 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/app/without-action/page.tsx @@ -0,0 +1,3 @@ +export default function Page() { + return <main>without-action</main> +} diff --git a/test/e2e/app-dir/action-forward-loop/next.config.js b/test/e2e/app-dir/action-forward-loop/next.config.js new file mode 100644 index 00000000000..807126e4cf0 --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/next.config.js @@ -0,0 +1,6 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = {} + +module.exports = nextConfig diff --git a/test/e2e/app-dir/action-forward-loop/proxy.ts b/test/e2e/app-dir/action-forward-loop/proxy.ts new file mode 100644 index 00000000000..a9f2b21f18a --- /dev/null +++ b/test/e2e/app-dir/action-forward-loop/proxy.ts @@ -0,0 +1,22 @@ +import { NextResponse } from 'next/server' +import type { NextRequest } from 'next/server' + +// Only rewrite action POSTs so the user can still GET the page to see the +// button. The receiving worker is /without-action, which does not bundle the +// action — so without the loop guard, the worker forwards the action back to +// /with-action, which is rewritten here again, and so on. +export function proxy(request: NextRequest) { + if ( + request.method === 'POST' && + request.nextUrl.pathname === '/with-action' + ) { + const url = request.nextUrl.clone() + url.pathname = '/without-action' + return NextResponse.rewrite(url) + } + return NextResponse.next() +} + +export const config = { + matcher: '/with-action', +}