Skip to content

decodeRedirectHeaderValue documents a resolved absolute target but validates nothing, so a javascript: or cross-origin target reaches the router #3175

Description

@frenzzy

Summary

decodeRedirectHeaderValue documents its output as "the author's status and the resolved absolute target", but it performs no validation — it splits on the first space and returns whatever follows. The absoluteness is a property of the server having resolved it, not of the decoder.

Tested against next @ a536e29b, built from source, Node 24.19.

"302 javascript:alert(document.cookie)"  ->  {"status":302,"url":"javascript:alert(document.cookie)"}
"302 https://evil.example/steal"         ->  {"status":302,"url":"https://evil.example/steal"}
"302 not-even-a-url"                     ->  {"status":302,"url":"not-even-a-url"}
"999 //evil.example"                     ->  {"status":999,"url":"//evil.example"}

The runtime emits it itself — correcting my own Scope section

I originally filed this as needing a hostile peer. That was wrong. maskRedirect resolves with new URL(target, requestUrl), and an absolute scheme wins over the base — so the classic open-redirect shape produces the javascript: header from ordinary application data:

registerServerFunction("login", async next => { throw redirect(next); });
"/dashboard"                          status=200  header: "302 http://localhost/dashboard"
"javascript:alert(document.cookie)"   status=200  header: "302 javascript:alert(document.cookie)"
"https://evil.example/steal"          status=200  header: "302 https://evil.example/steal"

redirect() does not validate the scheme, so throw redirect(url.searchParams.get("next")) is enough. No peer involved.

Scope

Core does not navigate; it decodes and hands the value to the router integration, which is the documented division. Nothing here is exploitable without an integration that navigates to the decoded value — but location.href = decoded.url is the obvious implementation, and on a javascript: target that is same-origin script execution.

The 999 row is peer-only: maskRedirect is reached only when the status is in validRedirectStatuses, so the runtime never emits a non-redirect status on this header.

What makes it worth raising is the gap between the stated contract and the enforced one. The docblock's neighbouring paragraph makes a point of the server-side resolution — "same-origin vs cross-origin is a real URL comparison, not a startsWith("http") coin toss" — and then the decoder is described as yielding "the resolved absolute target". A router author reading that reasonably concludes the value is safe to navigate to, and location.href = decoded.url on a javascript: target is same-origin script execution.

The reachable threat models are a compromised or malicious origin, and any intermediary that can inject a response header — which is precisely the position a CDN, a WAF, or a reverse proxy occupies.

Options

  1. Validate in the decoder — parse with new URL() and return undefined for a value that is not absolute or whose scheme is not http:/https:. Verified: the javascript:, not-even-a-url and 999 rows all become undefined, the same-origin control still decodes, the runtime-produced javascript: case is neutralised, and the full server suite is unchanged (48 files / 524 passed / 2 skipped).

    What it cannot do is reject https://evil.example/steal — the decoder has no request URL to compare origins against, and a cross-origin redirect is legitimate (redirect("https://checkout.stripe.com/…")). Closing that needs either the request URL passed in, or the origin decision left to the router, which is where it belongs.

  2. Return the parsed URL rather than a string. The type then carries the guarantee, and a router cannot accidentally use an unparsed value. Slightly larger API change; strictly more informative.

  3. Leave the decoder permissive and fix the docblock — state plainly that the value is whatever the peer sent, and that a reader must validate scheme and origin before navigating. Cheapest and honest; it moves the obligation to the integration where it currently lives.

  4. Validate on the status too999 is not a redirect status and currently passes through. Minor, but the same class: the field is documented as "the author's status" and any integer is accepted.

I'd suggest (1) plus (4), with the docblock amended to say what (1) does not cover, since the cross-origin case remains the router's call. If the position is that all validation belongs to the integration, then (3) is right and the docblock must say so explicitly — the current wording points the other way.

Separately, and possibly the earlier place to catch this: redirect() accepts a javascript: target at all. Happy to file that on its own if you'd rather keep this issue to the decoder.

Happy to send a PR for whichever direction you prefer, with a regression test covering the four rows above plus a valid same-origin absolute target as the control.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions