HMAC raw CSRF token before masking it, so it cannot be used to recons…

…truct a per-form token

[CVE-2020-8166]

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
 

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" }