From d124f19287f4892c72ca54da728a781591c6fca1 Mon Sep 17 00:00:00 2001
From: Jack McCracken <jack.mccracken@shopify.com>
Date: Wed, 13 May 2020 12:47:23 -0400
Subject: [PATCH] HMAC raw CSRF token before masking it, so it cannot be used
 to reconstruct a per-form token

[CVE-2020-8166]
---
 .../metal/request_forgery_protection.rb       | 33 ++++++++++++++++---
 .../request_forgery_protection_test.rb        | 33 +++++++++++++++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
index 4e097f305a5d..a64905e328cf 100644
--- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb
@@ -318,13 +318,15 @@ def masked_authenticity_token(session, form_options: {}) # :doc:
           action_path = normalize_action_path(action)
           per_form_csrf_token(session, action_path, method)
         else
-          real_csrf_token(session)
+          global_csrf_token(session)
         end
 
         one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
         encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token)
         masked_token = one_time_pad + encrypted_csrf_token
-        Base64.strict_encode64(masked_token)
+        Base64.urlsafe_encode64(masked_token, padding: false)
+
+        mask_token(raw_token)
       end
 
       # Checks the client's masked token to see if it matches the
@@ -354,7 +356,8 @@ def valid_authenticity_token?(session, encoded_masked_token) # :doc:
         elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2
           csrf_token = unmask_token(masked_token)
 
-          compare_with_real_token(csrf_token, session) ||
+          compare_with_global_token(csrf_token, session) ||
+            compare_with_real_token(csrf_token, session) ||
             valid_per_form_csrf_token?(csrf_token, session)
         else
           false # Token is malformed.
@@ -369,10 +372,21 @@ def unmask_token(masked_token) # :doc:
         xor_byte_strings(one_time_pad, encrypted_csrf_token)
       end
 
+      def mask_token(raw_token) # :doc:
+        one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
+        encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token)
+        masked_token = one_time_pad + encrypted_csrf_token
+        Base64.strict_encode64(masked_token)
+      end
+
       def compare_with_real_token(token, session) # :doc:
         ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session))
       end
 
+      def compare_with_global_token(token, session) # :doc:
+        ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, global_csrf_token(session))
+      end
+
       def valid_per_form_csrf_token?(token, session) # :doc:
         if per_form_csrf_tokens
           correct_token = per_form_csrf_token(
@@ -393,10 +407,21 @@ def real_csrf_token(session) # :doc:
       end
 
       def per_form_csrf_token(session, action_path, method) # :doc:
+        csrf_token_hmac(session, [action_path, method.downcase].join("#"))
+      end
+
+      GLOBAL_CSRF_TOKEN_IDENTIFIER = "!real_csrf_token"
+      private_constant :GLOBAL_CSRF_TOKEN_IDENTIFIER
+
+      def global_csrf_token(session) # :doc:
+        csrf_token_hmac(session, GLOBAL_CSRF_TOKEN_IDENTIFIER)
+      end
+
+      def csrf_token_hmac(session, identifier) # :doc:
         OpenSSL::HMAC.digest(
           OpenSSL::Digest::SHA256.new,
           real_csrf_token(session),
-          [action_path, method.downcase].join("#")
+          identifier
         )
       end
 
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index 7a02c27c9925..ac606c9d8f81 100644
--- a/actionpack/test/controller/request_forgery_protection_test.rb
+++ b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -907,6 +907,39 @@ def test_accepts_global_csrf_token
     assert_response :success
   end
 
+  def test_does_not_return_old_csrf_token
+    get :index
+
+    token = @controller.send(:form_authenticity_token)
+
+    unmasked_token = @controller.send(:unmask_token, Base64.urlsafe_decode64(token))
+
+    assert_not_equal @controller.send(:real_csrf_token, session), unmasked_token
+  end
+
+  def test_returns_hmacd_token
+    get :index
+
+    token = @controller.send(:form_authenticity_token)
+
+    unmasked_token = @controller.send(:unmask_token, Base64.urlsafe_decode64(token))
+
+    assert_equal @controller.send(:global_csrf_token, session), unmasked_token
+  end
+
+  def test_accepts_old_csrf_token
+    get :index
+
+    non_hmac_token = @controller.send(:mask_token, @controller.send(:real_csrf_token, session))
+
+    # This is required because PATH_INFO isn't reset between requests.
+    @request.env["PATH_INFO"] = "/per_form_tokens/post_one"
+    assert_nothing_raised do
+      post :post_one, params: { custom_authenticity_token: non_hmac_token }
+    end
+    assert_response :success
+  end
+
   def test_ignores_params
     get :index, params: { form_path: "/per_form_tokens/post_one?foo=bar" }