Make secure_compare handle empty strings comparison correctly

Used Rails' secure_compare method inside the definition of
secure_compare. This will handle the empty strings comparison and
return true when both the parameters are empty strings.

Fixes #4441, #4829

CHANGELOG.md

@@ -52,6 +52,7 @@
   * Use `OmniAuth.config.allowed_request_methods` as routing verbs for the auth path [#5508](https://github.com/heartcombo/devise/pull/5508)
   * Handle `on` and `ON` as true values to check params [#5514](https://github.com/heartcombo/devise/pull/5514)
   * Fix passing `format` option to `devise_for` [#5732](https://github.com/heartcombo/devise/pull/5732)
+  * Use `ActiveRecord::SecurityUtils.secure_compare` in `Devise.secure_compare` to match two empty strings correctly. [#4829](https://github.com/heartcombo/devise/pull/4829)
 
 
 Please check [4-stable](https://github.com/heartcombo/devise/blob/4-stable/CHANGELOG.md)

lib/devise.rb

@@ -517,12 +517,8 @@ module Devise
 
   # constant-time comparison algorithm to prevent timing attacks
   def self.secure_compare(a, b)
-    return false if a.blank? || b.blank? || a.bytesize != b.bytesize
-    l = a.unpack "C#{a.bytesize}"
-
-    res = 0
-    b.each_byte { |byte| res |= byte ^ l.shift }
-    res == 0
+    return false if a.nil? || b.nil?
+    ActiveSupport::SecurityUtils.secure_compare(a, b)
   end
 
   def self.deprecator

test/devise_test.rb

@@ -86,15 +86,20 @@ class DeviseTest < ActiveSupport::TestCase
     Devise::CONTROLLERS.delete(:kivi)
   end
 
-  test 'should complain when comparing empty or different sized passes' do
+  test 'Devise.secure_compare fails when comparing different strings or nil' do
     [nil, ""].each do |empty|
       assert_not Devise.secure_compare(empty, "something")
       assert_not Devise.secure_compare("something", empty)
-      assert_not Devise.secure_compare(empty, empty)
     end
+    assert_not Devise.secure_compare(nil, nil)
     assert_not Devise.secure_compare("size_1", "size_four")
   end
 
+  test 'Devise.secure_compare passes when strings are the same, even two empty strings' do
+    assert Devise.secure_compare("", "")
+    assert Devise.secure_compare("something", "something")
+  end
+
   test 'Devise.email_regexp should match valid email addresses' do
     valid_emails = ["test@example.com", "jo@jo.co", "f4$_m@you.com", "testing.example@example.com.ua", "test@tt", "test@valid---domain.com"]
     non_valid_emails = ["rex", "test user@example.com", "test_user@example server.com"]