Every OAuth callback has the same job. The user comes back from the identity provider, you exchange a code for a session, and then you send them where they were going. That last part is where the bug lives, and it is almost always written the same way.
const next = searchParams.get("next") ?? "/roadmap";
// ...exchange the code for a session...
return NextResponse.redirect(origin + next);This reads as safe. The origin is ours, it is hardcoded from the request, and next is only ever a path we put there ourselves. Two of those three statements are true.
The part that is not true
An attacker does not have to use the link you generated. They write their own, send it to someone, and let the browser do the rest. Concatenating a string you did not write onto an origin is not building a URL, it is hoping.
The interesting case is not the obvious one. Everyone thinks to block a value starting with https://. Try this instead.
/auth/callback?code=...&next=@evil.comConcatenate that onto https://sourcemap.co and you get https://sourcemap.co@evil.com. That is not a path on our site. Everything before the at sign is userinfo, which browsers accept and then discard, so the host is evil.com. The user watches a URL that starts with your domain take them somewhere else, having just authenticated.
There are more where that came from. Two leading slashes make it protocol-relative. A backslash is normalised to a forward slash by browsers but not by most naive checks. Each of these has its own blocklist entry, and maintaining that blocklist is the actual mistake.
Why string checks lose
The pattern that fails is checking what a string looks like. You are trying to answer a question about where a URL points, using a tool that only knows about characters. The blocklist grows every time somebody finds a new encoding, and you find out it was incomplete from a bug report.
The general shape: if you are writing checks against a string to answer a question about a structure, parse the structure and ask it.
The fix
Resolve the value as a URL against your own origin, then ask the parsed result which origin it belongs to. If it is not yours, discard it. Keep only the path and query, never the host, because the host is the entire question.
function safeNext(next: string | null, origin: string): string {
if (!next) return "/roadmap";
try {
const url = new URL(next, origin);
return url.origin === origin
? url.pathname + url.search
: "/roadmap";
} catch {
return "/roadmap";
}
}Four lines of logic and a catch. The catch matters: URL throws on input it cannot parse, and an exception inside a redirect handler is its own problem. Anything unparseable is not a destination we want anyway.
Run the payloads through it and every one of them lands on the same answer.
- @evil.com resolves to a path on our own host, so it is kept as a path and goes nowhere interesting
- //evil.com parses with evil.com as the origin, so it is discarded
- https://evil.com is discarded for the same reason
- A backslash variant is normalised by the parser before we ever compare, so it is discarded too
- javascript:alert(1) has a different origin, so it is discarded
Notice that none of those outcomes required knowing the payload existed. That is the whole point. A blocklist has to be told about the attack. A parser already knows, because answering that question is what it is for.
The part that is uncomfortable
This was live. Not for long, and on a site with no users yet, which is the only reason this post is easy to write. It was found by trying to break my own auth flow on purpose rather than by anything catching it automatically, and nothing in the type system, the linter or the build would have complained.
The lesson I took is not about redirects. It is that the code most likely to be wrong is the code that reads as obviously fine, and the reason it reads as fine is that you are looking at what it says rather than at what an attacker can send it.
If you want the rest of it
Sessions, tokens, and what actually happens in an OAuth callback are Day 39 and Day 40 of the ninety days. Free to read, like the other eighty-eight.