From 5e9973d6e020b98a5ec71578aa1837efcf4d7b7e Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
Date: Fri, 12 Feb 2021 12:59:54 -0600
Subject: [PATCH] Refactor CVE-2021-22881 fix

Follow-up to 83a6ac3fee8fd538ce7e0088913ff54f0f9bcb6f.

This allows `HTTP_HOST` to be omitted as before, and reduces the number
of object allocations per request.

Benchmark:

```ruby
 # frozen_string_literal: true
require "benchmark/memory"

HOST = "example.com:80"
BEFORE_REGEXP = /\A(?<host>[a-z0-9.-]+|\[[a-f0-9]*:[a-f0-9.:]+\])(:\d+)?\z/
AFTER_REGEXP = /(?:\A|,[ ]?)([a-z0-9.-]+|\[[a-f0-9]*:[a-f0-9.:]+\])(?::\d+)?\z/i

Benchmark.memory do |x|
  x.report("BEFORE (non-nil X-Forwarded-Host)") do
    origin_host = BEFORE_REGEXP.match(HOST.to_s.downcase)[:host]
    forwarded_host = BEFORE_REGEXP.match(HOST.to_s.split(/,\s?/).last)[:host]
  end

  x.report("BEFORE (nil X-Forwarded-Host)") do
    origin_host = BEFORE_REGEXP.match(HOST.to_s.downcase)[:host]
    forwarded_host = BEFORE_REGEXP.match(nil.to_s.split(/,\s?/).last)
  end

  x.report("AFTER (non-nil X-Forwarded-Host)") do
    origin_host = HOST&.slice(AFTER_REGEXP, 1) || ""
    forwarded_host = HOST&.slice(AFTER_REGEXP, 1) || ""
  end

  x.report("AFTER (nil X-Forwarded-Host)") do
    origin_host = HOST&.slice(AFTER_REGEXP, 1) || ""
    forwarded_host = nil&.slice(AFTER_REGEXP, 1) || ""
  end
end
```

Results:

```
BEFORE (non-nil X-Forwarded-Host)
                       616.000  memsize (   208.000  retained)
                         9.000  objects (     2.000  retained)
                         2.000  strings (     1.000  retained)
BEFORE (nil X-Forwarded-Host)
                       328.000  memsize (     0.000  retained)
                         5.000  objects (     0.000  retained)
                         2.000  strings (     0.000  retained)
AFTER (non-nil X-Forwarded-Host)
                       248.000  memsize (   168.000  retained)
                         3.000  objects (     1.000  retained)
                         1.000  strings (     0.000  retained)
AFTER (nil X-Forwarded-Host)
                        40.000  memsize (     0.000  retained)
                         1.000  objects (     0.000  retained)
                         1.000  strings (     0.000  retained)
```

[CVE-2021-22942]
---
 .../middleware/host_authorization.rb          | 20 +++++++------------
 .../test/dispatch/host_authorization_test.rb  |  6 +++---
 .../application/middleware/remote_ip_test.rb  |  1 -
 railties/test/isolation/abstract_unit.rb      |  2 +-
 4 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb
index e0fe9b33a0..62f0866f3e 100644
--- a/actionpack/lib/action_dispatch/middleware/host_authorization.rb
+++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb
@@ -102,21 +102,15 @@ 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)
-        valid_host = /
-          \A
-          (?<host>[a-z0-9.-]+|\[[a-f0-9]*:[a-f0-9.:]+\])
-          (:\d+)?
-          \z
-        /x
+        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 = valid_host.match(
-          request.get_header("HTTP_HOST").to_s.downcase)
-        forwarded_host = valid_host.match(
-          request.x_forwarded_host.to_s.split(/,\s?/).last)
-
-        origin_host && @permissions.allows?(origin_host[:host]) && (
-          forwarded_host.nil? || @permissions.allows?(forwarded_host[:host]))
+        @permissions.allows?(origin_host) && (forwarded_host.blank? || @permissions.allows?(forwarded_host))
       end
 
       def excluded?(request)
diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb
index ba5a443b5b..d6e366bb1a 100644
--- a/actionpack/test/dispatch/host_authorization_test.rb
+++ b/actionpack/test/dispatch/host_authorization_test.rb
@@ -221,15 +221,15 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     assert_match "Blocked host: www.example.com", response.body
   end
 
-  test "only compare to valid hostnames" do
+  test "blocks requests with invalid hostnames" do
     @app = ActionDispatch::HostAuthorization.new(App, ".example.com")
 
     get "/", env: {
-      "HOST" => "example.com#sub.example.com",
+      "HOST" => "attacker.com#x.example.com",
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: example.com#sub.example.com", response.body
+    assert_match "Blocked host: attacker.com#x.example.com", response.body
   end
 
   test "blocks requests to similar host" do
diff --git a/railties/test/application/middleware/remote_ip_test.rb b/railties/test/application/middleware/remote_ip_test.rb
index 0dc3dc3979..4c6fa1b371 100644
--- a/railties/test/application/middleware/remote_ip_test.rb
+++ b/railties/test/application/middleware/remote_ip_test.rb
@@ -11,7 +11,6 @@ class RemoteIpTest < ActiveSupport::TestCase
     def remote_ip(env = {})
       remote_ip = nil
       env = Rack::MockRequest.env_for("/").merge(env).merge!(
-        "HTTP_HOST" => "example.com",
         "action_dispatch.show_exceptions" => false,
         "action_dispatch.key_generator" => ActiveSupport::CachingKeyGenerator.new(
           ActiveSupport::KeyGenerator.new("b3c631c314c0bbca50c1b2843150fe33", iterations: 1000)
diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb
index 47fc313583..8a4d327285 100644
--- a/railties/test/isolation/abstract_unit.rb
+++ b/railties/test/isolation/abstract_unit.rb
@@ -82,7 +82,7 @@ def extract_body(response)
     end
 
     def get(path)
-      @app.call(::Rack::MockRequest.env_for(path, "HTTP_HOST" => "example.com"))
+      @app.call(::Rack::MockRequest.env_for(path))
     end
 
     def assert_welcome(resp)
-- 
2.30.2

