Skip to content

A TAB inside the scheme defeats the redirect scheme floor: java<TAB>script: ships and parses as javascript: #3201

Description

@frenzzy

Summary

refusedTargetScheme matches its scheme regex against the raw header value, but every URL parser strips ASCII TAB, LF and CR from a URL string before parsing. Inserting one TAB inside the scheme defeats the check: the target is not refused, ships verbatim, and is read back as the scheme it was supposed to block.

This defeats the navigation floor added for #3175 on the raw-Location road.

Tested against next @ 1cc2feb8, built from source, Node 24.19.

Reproduction

registerServerFunction("go", async target =>
  new Response(null, { status: 302, headers: { Location: target } })
);
plain  javascript:    status=500  Location: null                                    ← the floor works
TAB    java\tscript:  status=302  Location: "java\tscript:alert(document.cookie)"   → parses as javascript:
TAB    da\tta:        status=302  Location: "da\tta:text/html,x"                    → parses as data:
TAB    fi\tle:        status=302  Location: "fi\tle:///etc/passwd"                  → parses as file:
control /dashboard    status=302  Location: "/dashboard"                            → parses as http:

It generalises to every scheme the floor refuses — vbscript:, intent:, custom app schemes — and the TAB can sit anywhere inside the scheme token.

Cause

const match = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.exec(target);
return match !== null && !/^https?:$/i.test(match[0]);

Headers preserves HTAB (0x09) in a value verbatim, and TAB is not in [a-zA-Z0-9+.-], so the regex simply fails to match and the value is treated as scheme-less. The WHATWG URL parser removes all ASCII tab and newline characters before it begins, so it sees javascript: where the regex saw nothing.

Two of the three roads are unaffected, because they resolve through new URL() before the check reaches them:

road result
scripted / masked (maskRedirect) 500 — safe
no-JS (createNoJSHandler) 500 — safe
client decoder (decodeRedirectHeaderValue) undefined — safe
raw Location via enforceComposedHeaderInvariants bypassed

Only the road that reads the header raw is fooled — which is also the road documented as the "one check for every producer" backstop.

Scope

Not exploitable against a correctly-written app on its own. It needs an application-level open-redirect sink — request data reaching redirect() or a hand-built Location, the ?next= shape #3175's own comment names — plus a consumer that navigates the raw value.

What makes it worth fixing promptly anyway:

Fix

Resolve first, then judge the protocol — which is what the two safe roads already do:

function refusedTargetScheme(target, base) {
  let protocol;
  try { protocol = new URL(target, base).protocol; } catch { return true; }
  return protocol !== "http:" && protocol !== "https:";
}

Verified to preserve every documented allowance and close every bypass:

"/next"                   allowed   (unchanged)
"https://evil.com/next"   allowed   (unchanged — cross-origin http(s) is deliberate)
"//evil.com/next"         allowed   (unchanged)
"javascript:alert(1)"     refused   (unchanged)
"java\tscript:alert(1)"   refused   ← was allowed
"da\tta:text/html,x"      refused   ← was allowed

The alternative — stripping TAB/LF/CR before the existing regex — works too but re-implements URL normalization by hand, which is how this class of bug recurs.

Happy to send a PR with the fix plus a regression test covering the scheme matrix with and without embedded whitespace, on all three roads, with the relative and cross-origin allowances as controls.

A second bypassed road, and a second whitespace vector

The raw-Location road is not the only producer that reads a navigation header unparsed. An authored X-Server-Function-Redirect — set by an author, a transformResult hook, or a flight hook — goes through the same check, and on that road leading whitespace also gets through:

                        raw Location            authored X-Server-Function-Redirect
TAB interior            SHIPPED (bypassed)      SHIPPED (bypassed)
TAB before colon        SHIPPED (bypassed)      SHIPPED (bypassed)
leading SP              refused*                SHIPPED (bypassed)
leading TAB             refused*                SHIPPED (bypassed)
LF / CR anywhere        refused**               refused**

* not the floor — Headers.set trims leading whitespace from a value, so it never reaches the check.
** not the floor — Node's Headers rejects CR/LF in a value outright. A fix that relies on that is relying on the wrong layer.

enforceComposedHeaderInvariants slices the redirect header's target as everything after the first space, so leading whitespace sits interior to the header value and survives normalization.

Which roads are safe, and why: maskRedirect, createNoJSHandler and the client's decodeRedirectHeaderValue all resolve through new URL() before judging. Only the two that read raw are fooled.

Options

Five candidate implementations over the same target set. The naive reading of "resolve first" is a catastrophic regression and this table exists to stop it landing:

MUST REFUSE              today    B: new URL, no base    C: new URL + base    D: strip ws + regex    E: D + trim leading
javascript:x             refused  refused                refused              refused                refused
java<TAB>script:x        ALLOWED  refused                refused              refused                refused
javascript<TAB>:x        ALLOWED  refused                refused              refused                refused
" javascript:x"          ALLOWED  refused                refused              ALLOWED                refused
da<TAB>ta: / fi<TAB>le:  ALLOWED  refused                refused              refused                refused

MUST ALLOW               today    B                      C                    D                      E
/dashboard               allowed  *** REFUSED ***        allowed              allowed                allowed
dashboard                allowed  *** REFUSED ***        allowed              allowed                allowed
./dashboard:tab          allowed  *** REFUSED ***        allowed              allowed                allowed
"" / ?q=1 / #h           allowed  *** REFUSED ***        allowed              allowed                allowed
//evil.com/x             allowed  *** REFUSED ***        allowed              allowed                allowed
https://evil.com/x       allowed  allowed                allowed              allowed                allowed

B — resolve with no base: breaks 9 of 13 allowances, including every relative target, i.e. the ordinary case. enforceComposedHeaderInvariants has no request URL in hand today, so the base must be threaded in — that is the entire implementation cost of this fix and it is easy to miss.

C — resolve with the request URL as base. Recommended.

function refusedTargetScheme(target, base) {
  let protocol;
  try { protocol = new URL(target, base).protocol; } catch { return true; }
  return protocol !== "http:" && protocol !== "https:";
}

Closes every row, keeps every allowance, and makes this road do what the other three already do. One new URL() replaces a hand-written scheme grammar — smaller than the code it removes.

Behaviour change worth stating rather than discovering later: C also refuses six malformed-but-currently-allowed http(s) spellings (http://[bad, https://, //, https://exa mple.com/x, http://256.256.256.256/). None navigate anywhere, but a bare // is a plausible authored value and this answers 500, not a warning.

D — strip TAB/LF/CR, then the existing regex. Leaves the leading-space bypass on the redirect-header road open.

E — D plus a leading C0-control-or-space trim. Closes the matrix, but hand-rolls the URL spec's own normalization. That is how this class recurs: the next stripped code point or IDNA rule reopens it silently. The parser already knows the answer.

Do nothing. Worse than never shipping the floor — a one-character bypass in a control reads as covered.

Deliberate allowances the fix must not touch

Per the floor's own comment, cross-origin http(s) flows on purpose (OAuth hand-offs; the same-origin-vs-allowlist ruling is separate and pending), and mailto: and custom deep links are refused by default until that ruling gives them an opt-in. So "keep working" here includes "keep being refused" — do not add an allowance while fixing this.

Provenance

Introduced by e6372726fix: http(s) scheme floor for redirect navigation targets (#3175), which added refusedTargetScheme (packages/web/server-functions/src/server.ts:1504).

The same commit got the other two roads right by construction: maskRedirect and createNoJSHandler both resolve through new URL() before the value reaches the floor, and decodeRedirectHeaderValue parses too. Only enforceComposedHeaderInvariants reads the header raw, and that is the one the regex guards.

So the fix is not "add whitespace handling" — it is to make the third road do what the other two already do. That is also the smaller change: one new URL() replaces a hand-written scheme grammar.

Implemented and verified

Option C, with the base threaded from request.url — the piece that is easy to miss and the reason option B is a trap.

-function refusedTargetScheme(target) {
-  // explicit scheme present and not http(s) → refused
-  const match = /^[a-zA-Z][a-zA-Z0-9+.-]*:/.exec(target);
-  return match !== null && !/^https?:$/i.test(match[0]);
-}
+function refusedTargetScheme(target, base) {
+  // Judge the target the way a consumer will READ it, not the way its bytes
+  // are spelled: a URL parser strips ASCII tab and newline from anywhere in
+  // the string and trims leading C0/space before it begins, so a scheme
+  // grammar run over the raw value sees no scheme where the parser sees
+  // `javascript:` (#3201). Resolving is also what the masked and no-JS
+  // roads already do, which is why neither was ever fooled. The base keeps
+  // relative targets — the ordinary case — resolving to http(s).
+  let protocol;
+  try {
+    protocol = new URL(target, base).protocol;
+  } catch {
+    return true;
+  }
+  return protocol !== "http:" && protocol !== "https:";
+}

plus enforceComposedHeaderInvariants(response, base) and, at the one call site, enforceComposedHeaderInvariants(ownResponse(await dispatch()), request.url).

18 insertions, 7 deletions, seven of the insertions being the comment — so the executable change is roughly the size of the regex it replaces, and a hand-written scheme grammar disappears in favour of the parser that already decides the answer.

Verified

plain  javascript:   500, Location null   (unchanged)
TAB    java\tscript: 500, Location null   ← was 302, shipped, read as javascript:
TAB    da\tta:       500, Location null   ← was 302, read as data:
TAB    fi\tle:       500, Location null   ← was 302, read as file:

allowances, all still shipped:
  /dashboard   /d?n=1#t   dashboard   ./dashboard:tab   ""   ?q=1   #h
  https://app.example/x   http://app.example/x
  https://evil.com/x      //evil.com/x

The cross-origin and protocol-relative rows are the ones option B destroys; they survive here because of the base.

Full tracked suite with the change: 53 files, 586 passed, 2 skipped — no existing test moved.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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