Use string#split instead of regex for domain parts

[CVE-2023-22792]

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -296,20 +296,6 @@ def signed_cookie_digest
     class CookieJar # :nodoc:
       include Enumerable, ChainedCookieJars
 
-      # This regular expression is used to split the levels of a domain.
-      # The top level domain can be any string without a period or
-      # **.**, ***.** style TLDs like co.uk or com.au
-      #
-      # www.example.co.uk gives:
-      # $& => example.co.uk
-      #
-      # example.com gives:
-      # $& => example.com
-      #
-      # lots.of.subdomains.example.local gives:
-      # $& => example.local
-      DOMAIN_REGEXP = /[^.]*\.([^.]*|..\...|...\...)$/
-
       def self.build(req, cookies)
         jar = new(req)
         jar.update(cookies)
@@ -463,13 +449,35 @@ def handle_options(options)
           end
 
           if options[:domain] == :all || options[:domain] == "all"
-            # If there is a provided tld length then we use it otherwise default domain regexp.
-            domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP
+            cookie_domain = ""
+            dot_splitted_host = request.host.split('.', -1)
 
-            # If host is not ip and matches domain regexp.
-            # (ip confirms to domain regexp so we explicitly check for ip)
-            options[:domain] = if !request.host.match?(/^[\d.]+$/) && (request.host =~ domain_regexp)
-              ".#{$&}"
+            # Case where request.host is not an IP address or it's an invalid domain
+            # (ip confirms to the domain structure we expect so we explicitly check for ip)
+            if request.host.match?(/^[\d.]+$/) || dot_splitted_host.include?("") || dot_splitted_host.length == 1
+              options[:domain] = nil
+              return
+            end
+
+            # If there is a provided tld length then we use it otherwise default domain.
+            if options[:tld_length].present? 
+              # Case where the tld_length provided is valid
+              if dot_splitted_host.length >= options[:tld_length]
+                cookie_domain = dot_splitted_host.last(options[:tld_length]).join('.')
+              end
+            # Case where tld_length is not provided
+            else
+              # Regular TLDs
+              if !(/([^.]{2,3}\.[^.]{2})$/.match?(request.host))
+                cookie_domain = dot_splitted_host.last(2).join('.')
+              # **.**, ***.** style TLDs like co.uk and com.au
+              else
+                cookie_domain = dot_splitted_host.last(3).join('.')
+              end
+            end
+
+            options[:domain] = if cookie_domain.present?
+              ".#{cookie_domain}"
             end
           elsif options[:domain].is_a? Array
             # If host matches one of the supplied domains.

actionpack/test/dispatch/cookies_test.rb

@@ -257,6 +257,11 @@ def set_cookie_with_domain_and_tld
       head :ok
     end
 
+    def set_cookie_with_domain_and_longer_tld
+      cookies[:user_name] = { value: "rizwanreza", domain: :all, tld_length: 4 }
+      head :ok 
+    end
+
     def delete_cookie_with_domain_and_tld
       cookies.delete(:user_name, domain: :all, tld_length: 2)
       head :ok
@@ -1132,6 +1137,13 @@ def test_cookie_with_all_domain_option_using_australian_style_tld
     assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com.au; path=/; SameSite=Lax"
   end
 
+  def test_cookie_with_all_domain_option_using_australian_style_tld_and_two_subdomains
+    @request.host = "x.nextangle.com.au"
+    get :set_cookie_with_domain
+    assert_response :success
+    assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com.au; path=/; SameSite=Lax"
+  end
+
   def test_cookie_with_all_domain_option_using_uk_style_tld
     @request.host = "nextangle.co.uk"
     get :set_cookie_with_domain
@@ -1139,6 +1151,13 @@ def test_cookie_with_all_domain_option_using_uk_style_tld
     assert_cookie_header "user_name=rizwanreza; domain=.nextangle.co.uk; path=/; SameSite=Lax"
   end
 
+  def test_cookie_with_all_domain_option_using_uk_style_tld_and_two_subdomains
+    @request.host = "x.nextangle.co.uk"
+    get :set_cookie_with_domain
+    assert_response :success
+    assert_cookie_header "user_name=rizwanreza; domain=.nextangle.co.uk; path=/; SameSite=Lax"
+  end
+
   def test_cookie_with_all_domain_option_using_host_with_port
     @request.host = "nextangle.local:3000"
     get :set_cookie_with_domain
@@ -1201,6 +1220,13 @@ def test_cookie_with_all_domain_option_using_host_with_port_and_tld_length
     assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/; SameSite=Lax"
   end
 
+  def test_cookie_with_all_domain_option_using_longer_tld_length
+    @request.host = "x.y.z.t.com"
+    get :set_cookie_with_domain_and_longer_tld
+    assert_response :success
+    assert_cookie_header "user_name=rizwanreza; domain=.y.z.t.com; path=/; SameSite=Lax"
+  end
+
   def test_deleting_cookie_with_all_domain_option_and_tld_length
     request.cookies[:user_name] = "Joe"
     get :delete_cookie_with_domain_and_tld