From 970d3b2e7387c34663988b0db222d045c34c59c1 Mon Sep 17 00:00:00 2001
From: Stef Schenkelaars <stef.schenkelaars@gmail.com>
Date: Wed, 7 Jul 2021 12:06:32 +0200
Subject: [PATCH] Fix invalid forwarded host vulnerability

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]
---
 .../middleware/host_authorization.rb          | 10 +--
 .../test/dispatch/host_authorization_test.rb  | 89 ++++++++++++++++++-
 2 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb
index 968e862800..d21bca72c6 100644
--- a/actionpack/lib/action_dispatch/middleware/host_authorization.rb
+++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb
@@ -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
@@ -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
diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb
index b98b6492a0..85cb6c7318 100644
--- a/actionpack/test/dispatch/host_authorization_test.rb
+++ b/actionpack/test/dispatch/host_authorization_test.rb
@@ -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)
 
@@ -205,11 +243,23 @@ 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",
     }
 
@@ -217,6 +267,43 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     assert_equal "Success", body
   end
 
+  test "lots of NG hosts" do
+    ng_hosts = [
+      "hacker%E3%80%82com",
+      "hacker%00.com",
+      "www.theirsite.com@yoursite.com",
+      "hacker.com/test/",
+      "hacker%252ecom",
+      ".hacker.com",
+      "/\/\/hacker.com/",
+      "/hacker.com",
+      "../hacker.com",
+      ".hacker.com",
+      "@hacker.com",
+      "hacker.com",
+      "hacker.com%23@example.com",
+      "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" })
 
-- 
2.33.0