Validate X-Forwarded-Host before using it as a post-login redirect target
render_unauthorized built the post-login return URL directly from the attacker-influenceable X-Forwarded-Host / X-Forwarded-Uri headers, stored it in the session, and reflected it into the signin `rd`. After authentication that URL is followed with allow_other_host, so a spoofed host was an open redirect. Now the forwarded URL is only honoured if it resolves to a known, active forward-auth application (via validate_redirect_url); otherwise it falls back to a validated `rd` or the IdP's base URL. Once render_unauthorized only ever stores a validated value, the sessions_controller "supplement, don't replace" behaviour is safe, so no change is needed there. Two integration tests were asserting the old behaviour by reflecting unregistered hosts (grafana.example.com, app.example.com); they now register those domains as forward-auth apps so they exercise the real feature. Adds a regression test that a spoofed X-Forwarded-Host is neither stored nor reflected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -196,11 +196,14 @@ module Api
|
||||
original_host = request.headers["X-Forwarded-Host"]
|
||||
original_uri = request.headers["X-Forwarded-Uri"] || request.headers["X-Forwarded-Path"] || "/"
|
||||
|
||||
original_url = if original_host
|
||||
"https://#{original_host}#{original_uri}"
|
||||
else
|
||||
redirect_url || base_url
|
||||
end
|
||||
# X-Forwarded-Host is attacker-influenceable, so only honour the forwarded
|
||||
# URL as a post-login redirect target if it resolves to a known, active
|
||||
# forward-auth application. Otherwise this is an open redirect: a spoofed
|
||||
# host would be stored and reflected into the signin `rd`, then followed
|
||||
# (with allow_other_host) after the user authenticates. Fall back to a
|
||||
# validated `rd` or, failing that, the IdP's own base URL.
|
||||
forwarded_url = "https://#{original_host}#{original_uri}" if original_host.present?
|
||||
original_url = validate_redirect_url(forwarded_url) || redirect_url || base_url
|
||||
|
||||
session[:return_to_after_authenticating] = original_url
|
||||
|
||||
|
||||
@@ -153,6 +153,10 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
|
||||
|
||||
# Redirect URL Integration Tests
|
||||
test "unauthenticated request redirects to signin with parameters" do
|
||||
# grafana.example.com must be a registered forward-auth app for its URL to be
|
||||
# honoured as a redirect target (otherwise it would be an open-redirect vector).
|
||||
grant_everyone_access Application.create!(name: "Grafana", slug: "grafana", app_type: "forward_auth", domain_pattern: "grafana.example.com", active: true)
|
||||
|
||||
# Test that unauthenticated requests redirect to signin with rd and rm parameters
|
||||
get "/api/verify", headers: {
|
||||
"X-Forwarded-Host" => "grafana.example.com"
|
||||
@@ -172,7 +176,32 @@ class ForwardAuthIntegrationTest < ActionDispatch::IntegrationTest
|
||||
assert_includes location, "grafana.example.com"
|
||||
end
|
||||
|
||||
test "spoofed X-Forwarded-Host is not reflected as a redirect target" do
|
||||
# CLINCH_HOST pins the IdP origin (as in production) so base_url cannot be
|
||||
# influenced by the request; this isolates the return_to/redirect behaviour.
|
||||
original_clinch_host = ENV["CLINCH_HOST"]
|
||||
ENV["CLINCH_HOST"] = "https://auth.example.com"
|
||||
|
||||
# No forward-auth app exists for evil.com, and no valid rd is supplied. The
|
||||
# attacker-controlled host must NOT be stored or reflected into the signin URL.
|
||||
get "/api/verify", headers: {
|
||||
"X-Forwarded-Host" => "evil.com",
|
||||
"X-Forwarded-Uri" => "/steal"
|
||||
}
|
||||
|
||||
assert_response 302
|
||||
assert_match %r{/signin}, response.location
|
||||
refute_includes response.location, "evil.com"
|
||||
refute_match(/evil\.com/, session[:return_to_after_authenticating].to_s)
|
||||
ensure
|
||||
ENV["CLINCH_HOST"] = original_clinch_host
|
||||
end
|
||||
|
||||
test "return URL functionality after authentication" do
|
||||
# app.example.com must be a registered forward-auth app for its URL to be
|
||||
# honoured as a redirect target.
|
||||
grant_everyone_access Application.create!(name: "App FA", slug: "app-fa", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
|
||||
|
||||
# Initial request should set return URL
|
||||
get "/api/verify", headers: {
|
||||
"X-Forwarded-Host" => "app.example.com",
|
||||
|
||||
Reference in New Issue
Block a user