Skip to content

Commit b71bb8b

Browse files
committed
evil host redirect hanlding for pentest
1 parent 8971492 commit b71bb8b

2 files changed

Lines changed: 88 additions & 13 deletions

File tree

admin/server/auth/handlers.go

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010
"net/url"
1111
"strconv"
12+
"strings"
1213
"time"
1314

1415
"github.com/coreos/go-oidc/v3/oidc"
@@ -166,26 +167,54 @@ func (a *Authenticator) authStart(w http.ResponseWriter, r *http.Request, signup
166167
// Set state in cookie
167168
sess.Values[cookieFieldState] = state
168169

169-
// Set redirect URL in cookie to enable custom redirects after auth has completed
170+
host := originalHost(r)
171+
172+
// Parse custom_domain_flow early — needed to gate the DB lookup for redirect validation.
173+
customDomainFlow := false
174+
if b, err := strconv.ParseBool(r.URL.Query().Get("custom_domain_flow")); err == nil {
175+
customDomainFlow = b
176+
}
177+
if customDomainFlow {
178+
sess.Values[cookieFieldCustomDomainFlow] = true
179+
}
180+
181+
// Validate and store the redirect URL.
170182
redirect := r.URL.Query().Get("redirect")
171183
if redirect != "" {
184+
if !a.admin.URLs.IsSafeRedirectURL(redirect, host) {
185+
// The redirect is not on a primary/canonical host and not on the request host.
186+
// The only legitimate case is the server-generated second call in the custom
187+
// domain login flow (canonical domain, custom_domain_flow=true), where the
188+
// redirect points to <custom-domain>/auth/custom-domain-callback.
189+
if !customDomainFlow {
190+
http.Error(w, "invalid redirect parameter", http.StatusBadRequest)
191+
return
192+
}
193+
parsed, _ := url.Parse(redirect)
194+
_, err := a.admin.DB.FindOrganizationByCustomDomain(r.Context(), parsed.Host)
195+
if errors.Is(err, database.ErrNotFound) {
196+
http.Error(w, "invalid redirect parameter", http.StatusBadRequest)
197+
return
198+
} else if err != nil {
199+
http.Error(w, "internal server error", http.StatusInternalServerError)
200+
return
201+
}
202+
// Path must be the custom-domain callback endpoint (with or without /api prefix).
203+
if !strings.HasSuffix(parsed.Path, "/auth/custom-domain-callback") {
204+
http.Error(w, "invalid redirect parameter", http.StatusBadRequest)
205+
return
206+
}
207+
}
172208
sess.Values[cookieFieldRedirect] = redirect
173209
}
174210

175-
// If this is part of the custom domain login flow, save that info in the cookie since we need that info when handling the auth callback.
176-
customDomainFlow := r.URL.Query().Get("custom_domain_flow")
177-
if b, err := strconv.ParseBool(customDomainFlow); err == nil && b {
178-
sess.Values[cookieFieldCustomDomainFlow] = true
179-
}
180-
181211
// Save cookie
182212
if err := sess.Save(r, w); err != nil {
183213
http.Error(w, fmt.Sprintf("failed to save session: %s", err), http.StatusInternalServerError)
184214
return
185215
}
186216

187217
// Redirect to <canonical-domain>/auth/login (custom domain flow)
188-
host := originalHost(r)
189218
if a.admin.URLs.IsCustomDomain(host) {
190219
customCallbackURL := a.admin.URLs.WithCustomDomain(host).AuthCustomDomainCallback(state)
191220
canonicalLoginURL := a.admin.URLs.AuthLogin(customCallbackURL, true)
@@ -569,8 +598,12 @@ func (a *Authenticator) authLogout(w http.ResponseWriter, r *http.Request) {
569598
return
570599
}
571600

572-
// Extract custom redirect destination (if any)
601+
// Extract and validate custom redirect destination (if any).
573602
redirect := r.URL.Query().Get("redirect")
603+
if redirect != "" && !a.admin.URLs.IsSafeRedirectURL(redirect, originalHost(r)) {
604+
http.Error(w, "invalid redirect parameter", http.StatusBadRequest)
605+
return
606+
}
574607

575608
// Redirect to authLogoutProvider (see its docstring below for details on why we do this).
576609
http.Redirect(w, r, a.admin.URLs.AuthLogoutProvider(redirect), http.StatusTemporaryRedirect)
@@ -580,14 +613,23 @@ func (a *Authenticator) authLogout(w http.ResponseWriter, r *http.Request) {
580613
// This is separated from authLogout to support orgs with custom domains where the auth token cookie must be cleared from the custom domain,
581614
// but the redirect destination must be set in a cookie on the primary domain because the auth provider will redirect to authLogoutCallback on the primary domain.
582615
func (a *Authenticator) authLogoutProvider(w http.ResponseWriter, r *http.Request) {
583-
// Set custom redirect destination in cookie for when the logout flow is over (if any)
616+
// Validate and store the custom redirect destination for when the logout flow is over (if any).
584617
redirect := r.URL.Query().Get("redirect")
585618
if redirect != "" {
586-
// Update cookie
619+
if !a.admin.URLs.IsSafeRedirectURL(redirect, "") {
620+
// Not a primary host — check if it's a registered Rill custom domain.
621+
parsed, _ := url.Parse(redirect)
622+
_, err := a.admin.DB.FindOrganizationByCustomDomain(r.Context(), parsed.Host)
623+
if errors.Is(err, database.ErrNotFound) {
624+
http.Error(w, "invalid redirect parameter", http.StatusBadRequest)
625+
return
626+
} else if err != nil {
627+
http.Error(w, "internal server error", http.StatusInternalServerError)
628+
return
629+
}
630+
}
587631
sess := a.cookies.Get(r, cookieName)
588632
sess.Values[cookieFieldRedirect] = redirect
589-
590-
// Save cookie
591633
if err := sess.Save(r, w); err != nil {
592634
http.Error(w, fmt.Sprintf("failed to save session: %s", err), http.StatusInternalServerError)
593635
return

admin/urls.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,39 @@ func (u *URLs) WithCustomDomain(domain string) *URLs {
7979
}
8080
}
8181

82+
// IsSafeRedirectURL reports whether redirect is safe to redirect to after an auth flow.
83+
// A redirect is safe when it is:
84+
// - empty (caller defaults to the frontend URL)
85+
// - a relative path (no scheme, no host); protocol-relative "//evil.com" and
86+
// scheme-only "javascript:" / "data:" forms are rejected
87+
// - an absolute URL whose host matches the primary external URL host, the primary
88+
// frontend URL host, or the optionally supplied additionalHost (e.g. the custom
89+
// domain of the current request)
90+
func (u *URLs) IsSafeRedirectURL(redirect, additionalHost string) bool {
91+
if redirect == "" {
92+
return true
93+
}
94+
parsed, err := url.Parse(redirect)
95+
if err != nil {
96+
return false
97+
}
98+
// No host: safe only when there is also no scheme.
99+
// This rejects javascript:, data:, mailto:, and //evil.com forms.
100+
if parsed.Host == "" {
101+
return parsed.Scheme == ""
102+
}
103+
// Absolute URL: host must match a trusted host.
104+
externalURL, err := url.Parse(u.external)
105+
if err == nil && strings.EqualFold(parsed.Host, externalURL.Host) {
106+
return true
107+
}
108+
frontendURL, err := url.Parse(u.frontend)
109+
if err == nil && strings.EqualFold(parsed.Host, frontendURL.Host) {
110+
return true
111+
}
112+
return additionalHost != "" && strings.EqualFold(parsed.Host, additionalHost)
113+
}
114+
82115
// WithCustomDomainFromRedirectURL attempts to infer a custom domain from a redirect URL.
83116
// If it succeeds, it passes the custom domain to WithCustomDomain and returns the result.
84117
// If it does not detect a custom domain in the redirect URL, or the redirect URL is invalid, it fails silently by returning itself unchanged.

0 commit comments

Comments
 (0)