Skip to content

Commit

Permalink
Fix invalid forwarded host vulnerability
Browse files Browse the repository at this point in the history
Prior to this commit, it was possible to pass an unvalidated host
through the `X-Forwarded-Host` header. If the value of the header
was prefixed with a invalid domain character (for example a `/`),
it was always accepted as the actual host of that request.

Since this host is used for all url helpers, an attacker could change
generated links and redirects. If the header is set to
`X-Forwarded-Host: //evil.hacker`, a redirect will be send to
`https:////evil.hacker/`. Browsers will ignore these four slashes
and redirect the user.

[CVE-2021-44528]
  • Loading branch information
StefSchenkelaars authored and tenderlove committed Dec 14, 2021
1 parent d12eca1 commit 0fccfb9
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 8 deletions.
10 changes: 3 additions & 7 deletions actionpack/lib/action_dispatch/middleware/host_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def sanitize_regexp(host)

def sanitize_string(host)
if host.start_with?(".")
/\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/i
/\A([a-z0-9-]+\.)?#{Regexp.escape(host[1..-1])}\z/i
else
/\A#{Regexp.escape host}\z/i
end
Expand Down Expand Up @@ -120,13 +120,9 @@ def call(env)
end

private
HOSTNAME = /[a-z0-9.-]+|\[[a-f0-9]*:[a-f0-9.:]+\]/i
VALID_ORIGIN_HOST = /\A(#{HOSTNAME})(?::\d+)?\z/
VALID_FORWARDED_HOST = /(?:\A|,[ ]?)(#{HOSTNAME})(?::\d+)?\z/

def authorized?(request)
origin_host = request.get_header("HTTP_HOST")&.slice(VALID_ORIGIN_HOST, 1) || ""
forwarded_host = request.x_forwarded_host&.slice(VALID_FORWARDED_HOST, 1) || ""
origin_host = request.get_header("HTTP_HOST")
forwarded_host = request.x_forwarded_host&.split(/,\s?/)&.last

@permissions.allows?(origin_host) && (forwarded_host.blank? || @permissions.allows?(forwarded_host))
end
Expand Down
89 changes: 88 additions & 1 deletion actionpack/test/dispatch/host_authorization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,44 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
assert_match "Blocked host: 127.0.0.1", response.body
end

test "blocks requests with spoofed relative X-FORWARDED-HOST" do
@app = ActionDispatch::HostAuthorization.new(App, ["www.example.com"])

get "/", env: {
"HTTP_X_FORWARDED_HOST" => "//randomhost.com",
"HOST" => "www.example.com",
"action_dispatch.show_detailed_exceptions" => true
}

assert_response :forbidden
assert_match "Blocked host: //randomhost.com", response.body
end

test "forwarded secondary hosts are allowed when permitted" do
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")

get "/", env: {
"HTTP_X_FORWARDED_HOST" => "example.com, my-sub.domain.com",
"HOST" => "domain.com",
}

assert_response :ok
assert_equal "Success", body
end

test "forwarded secondary hosts are blocked when mismatch" do
@app = ActionDispatch::HostAuthorization.new(App, "domain.com")

get "/", env: {
"HTTP_X_FORWARDED_HOST" => "domain.com, evil.com",
"HOST" => "domain.com",
"action_dispatch.show_detailed_exceptions" => true
}

assert_response :forbidden
assert_match "Blocked host: evil.com", response.body
end

test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do
@app = ActionDispatch::HostAuthorization.new(App, nil)

Expand Down Expand Up @@ -205,18 +243,67 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
assert_match "Blocked host: sub.domain.com", response.body
end

test "sub-sub domains should not be permitted" do
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")

get "/", env: {
"HOST" => "secondary.sub.domain.com",
"action_dispatch.show_detailed_exceptions" => true
}

assert_response :forbidden
assert_match "Blocked host: secondary.sub.domain.com", response.body
end

test "forwarded hosts are allowed when permitted" do
@app = ActionDispatch::HostAuthorization.new(App, ".domain.com")

get "/", env: {
"HTTP_X_FORWARDED_HOST" => "sub.domain.com",
"HTTP_X_FORWARDED_HOST" => "my-sub.domain.com",
"HOST" => "domain.com",
}

assert_response :ok
assert_equal "Success", body
end

test "lots of NG hosts" do
ng_hosts = [
"hacker%E3%80%82com",
"hacker%00.com",
"[email protected]",
"hacker.com/test/",
"hacker%252ecom",
".hacker.com",
"/\/\/hacker.com/",
"/hacker.com",
"../hacker.com",
".hacker.com",
"@hacker.com",
"hacker.com",
"hacker.com%[email protected]",
"hacker.com/.jpg",
"hacker.com\texample.com/",
"hacker.com/example.com",
"hacker.com\@example.com",
"hacker.com/example.com",
"hacker.com/"
]

@app = ActionDispatch::HostAuthorization.new(App, "example.com")

ng_hosts.each do |host|
get "/", env: {
"HTTP_X_FORWARDED_HOST" => host,
"HOST" => "example.com",
"action_dispatch.show_detailed_exceptions" => true
}

assert_response :forbidden
assert_match "Blocked host: #{host}", response.body
end
end

test "exclude matches allow any host" do
@app = ActionDispatch::HostAuthorization.new(App, "only.com", exclude: ->(req) { req.path == "/foo" })

Expand Down

0 comments on commit 0fccfb9

Please sign in to comment.