diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb index 5e455fca5775e..2693f3440c645 100644 --- a/activesupport/lib/active_support/security_utils.rb +++ b/activesupport/lib/active_support/security_utils.rb @@ -19,12 +19,14 @@ def fixed_length_secure_compare(a, b) end module_function :fixed_length_secure_compare - # Constant time string comparison, for variable length strings. + # Secure string comparison for strings of variable length. # - # The values are first processed by SHA256, so that we don't leak length info - # via timing attacks. + # While a timing attack would not be able to discern the content of + # a secret compared via secure_compare, it is possible to determine + # the secret length. This should be considered when using secure_compare + # to compare weak, short secrets to user input. def secure_compare(a, b) - fixed_length_secure_compare(::Digest::SHA256.digest(a), ::Digest::SHA256.digest(b)) && a == b + a.length == b.length && fixed_length_secure_compare(a, b) end module_function :secure_compare end