mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): harden oauth2 redirect validation (#27274)
Closes DEVEX-604 Hardens `redirect` URL handling in the OAuth2/OIDC/external-auth callback flows so redirects are always reduced to a safe, relative path local to the application. Previously a redirect value with an opaque scheme (e.g. `javascript:...`) or a path with multiple leading slashes (e.g. `///evil.com`) could survive sanitization mostly intact. Also de-duplicates the previously copy-pasted `uriFromURL` helper (now exported `httpmw.URIFromURL`) so there's a single implementation shared by `coderd/userauth.go`, `coderd/externalauth.go`, and `coderd/httpmw/oauth2.go`. <details> <summary>Context</summary> Addresses a low-severity finding reported via a pentest disclosure: the redirect sanitizer used `url.Parse(...).RequestURI()`, which doesn't reject non-hierarchical (opaque) URLs and doesn't collapse extra leading slashes, so crafted `redirect` values could partially survive sanitization. </details> This PR was authored by a Coder Agent on behalf of @aslilac.
This commit is contained in:
+1
-10
@@ -133,7 +133,7 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg
|
||||
// the host of the AccessURL but ultimately as long as our redirect
|
||||
// url omits a host we're ensuring that we're routing to a path
|
||||
// local to the application.
|
||||
redirect = uriFromURL(redirect)
|
||||
redirect = httpapi.SafeRedirectPath(redirect)
|
||||
}
|
||||
|
||||
// When dynamic redirect URIs are enabled, validate the request Host
|
||||
@@ -532,15 +532,6 @@ func ExtractOAuth2ProviderAppSecret(db database.Store) func(http.Handler) http.H
|
||||
}
|
||||
}
|
||||
|
||||
func uriFromURL(u string) string {
|
||||
uri, err := url.Parse(u)
|
||||
if err != nil {
|
||||
return "/"
|
||||
}
|
||||
|
||||
return uri.RequestURI()
|
||||
}
|
||||
|
||||
// buildDynamicRedirectURI constructs the OIDC redirect_uri from the incoming
|
||||
// request, used when CODER_OIDC_REDIRECT_ALLOWED_HOSTS is configured.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user