fix(console): validate continue param in /auth/authorize to prevent open redirect (CWE-601)#26373
fix(console): validate continue param in /auth/authorize to prevent open redirect (CWE-601)#26373sebastiondev wants to merge 1 commit intoanomalyco:devfrom
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Thanks — I've updated the PR description to follow the template (Type of change, What does this PR do, How did you verify, Screenshots, Checklist all filled in). On the linked-issue point: this PR fixes a security vulnerability (CWE-601, Open Redirect) and I deliberately didn't open a public issue first to avoid publishing exploit details before a fix landed. I've explained that in the new "Issue for this PR" section. If maintainers would prefer this go through a private security channel (security advisory / SECURITY.md contact) instead, I'm happy to close this and resubmit there — please advise. |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
Issue for this PR
No public tracking issue — this PR fixes a security vulnerability (CWE-601, Open Redirect) and we did not want to publish exploit details in a public issue before a fix landed. Happy to open one retroactively, or to redirect this through a private security channel if maintainers prefer; please advise.
Type of change
What does this PR do?
The console's
/auth/authorizeroute concatenates the user-suppliedcontinuequery parameter directly into the OAuth callback URL without validation. After the OAuth round-trip, the callback handler derives a redirect target from the callback path, which means a craftedcontinuevalue can redirect the user to an attacker-controlled origin once authentication completes.Affected file:
packages/console/app/src/routes/auth/authorize.tsData flow:
/auth/authorize?continue=<X>readscontinuefrom the query string.callbackUrl = new URL("./callback" + cont, request.url)and passes it toAuthClient.authorize(...)as the OAuthredirect_uri.[...callback]computesnext = url.pathname.replace("/auth/callback","")and callsredirect(route(locale, next)).route()returnsnextverbatim for paths it doesn't specifically rewrite, so anextof//evil.com/xis passed through.Location: //evil.com/xresponse ashttps://evil.com/x— the user is redirected off-site.A payload like
/auth/authorize?continue=//evil.com/phishis sufficient. The attacker doesn't need any privileges; the victim only needs to click the link.The fix adds a
safeContinue()validator at the entry point that only accepts values which:/(relative path, not protocol-relative),//,\, or/\(protocol-relative / backslash variants that some browsers normalize),.., backslashes, or CR/LF/TAB characters.Anything else is replaced with an empty string, which produces the safe default callback URL
/auth/callback. The change is 18 lines, scoped to the single vulnerable file, and preserves the legitimate behavior ofcontinuefor in-app post-login navigation.Why this works: the vulnerable primitive is
"./callback" + contresolved againstrequest.url. By rejecting anycontthat isn't a single-slash relative path with no traversal/backslash/control characters, the resolvedcallbackUrlcan no longer escape the/auth/callbackprefix on the legitimate origin, so thenextderived later in the callback handler can no longer be turned into a protocol-relative or off-origin redirect.How did you verify your code works?
Walked the validator against each attack class and the legitimate inputs by hand:
""""/workspace/abc/workspace/abc//evil.com/x""/\evil.com""\\evil.com""/foo/../../bar""https://evil.com""/)""The rest of the auth flow is untouched, so existing OAuth behavior for valid relative
continuevalues is preserved. I did not add an automated unit test because the surrounding routes don't currently have unit-test coverage in this package; happy to add one if maintainers point me at the preferred test harness.Adversarial review before submitting: checked whether SolidStart,
route(), or the surrounding framework normalizes protocol-relative URLs before redirecting — they don't. Checked whether the OAuth IdP would always reject the manipulatedredirect_uri— that depends on IdP configuration and isn't guaranteed (OpenAuth-style IdPs commonly accept path-prefixredirect_urivalues). Checked whether there were existing access controls on/auth/authorize— there aren't; it's an unauthenticated GET.Screenshots / recordings
N/A — no UI change.
Checklist