From aecba3c301b80e9d5a63c30ea1b287bceaf2c107 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 62f0866f3e..93610e5097 100644
--- a/actionpack/lib/action_dispatch/middleware/host_authorization.rb
+++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb
@@ -51,7 +51,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
@@ -102,13 +102,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 d6e366bb1a..ff0a6df503 100644
--- a/actionpack/test/dispatch/host_authorization_test.rb
+++ b/actionpack/test/dispatch/host_authorization_test.rb
@@ -155,6 +155,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)
 
@@ -191,11 +229,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",
     }
 
@@ -203,6 +253,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