Skip to content

fix: prevent open redirect in OAuth callback (CWE-601)#15

Closed
nicknisi wants to merge 1 commit intomainfrom
fix/open-redirect-callback
Closed

fix: prevent open redirect in OAuth callback (CWE-601)#15
nicknisi wants to merge 1 commit intomainfrom
fix/open-redirect-callback

Conversation

@nicknisi
Copy link
Copy Markdown
Member

Summary

  • handleCallback was writing the OAuth state's returnPathname directly into the Location header. Because state round-trips through the IdP and is attacker-influenceable, crafted values (https://evil.com, //evil.com, /\evil.com) redirected victims off-origin after a successful sign-in — a credential-phishing primitive (CWE-601).
  • Ports the authkit-nextjs mitigation: parse the untrusted returnPathname against a throwaway origin so the WHATWG URL parser strips any host, scheme, or backslash trick, then emit a relative Location built from pathname + search + hash with a leading-slash normalization that defuses the //evil.com protocol-relative case.
  • Relative (not absolute) Location avoids a deployment footgun: behind proxies/TLS terminators that don't rewrite event.url's origin, an absolute Location would leak an internal backend host. The browser resolves the relative path against the public callback URL instead.
  • Hash fragments preserved, so /dashboard#billing still lands on the right anchor / client-side route.

`handleCallback` wrote the OAuth state's `returnPathname` straight into
a `Location` header. The state is attacker-influenceable, so crafted
values (`https://evil.com`, `//evil.com`, `/\evil.com`) let an attacker
redirect victims off-origin after a successful sign-in — a classic
credential-phishing primitive.

Port the authkit-nextjs mitigation: parse `returnPathname` against a
throwaway origin so the WHATWG URL parser strips host/scheme/backslash
tricks, then emit a RELATIVE Location (`pathname + search + hash`) with
a leading-slash normalization that kills the `//evil.com` protocol-
relative edge case. Relative (not absolute) keeps us safe behind proxies
that don't reconstruct `event.url`'s public origin, and the hash
fragment is preserved so `/dashboard#billing` still lands correctly.

Adds 10 tests covering attack payloads, query/hash preservation, and a
proxied-origin scenario.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant