From fa487763d98ccf9c3e66fdb44f09af5c37a50fe5 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Tue, 12 Apr 2016 02:41:06 +0530 Subject: Changed default behaviour of `ActiveSupport::SecurityUtils.secure_compare`, to make it not leak length information even for variable length string. Renamed old `ActiveSupport::SecurityUtils.secure_compare` to `fixed_length_secure_compare`, and started raising `ArgumentError` in case of length mismatch of passed strings. --- activesupport/lib/active_support/security_utils.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'activesupport/lib') diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb index b655d64449..5f5d4faa60 100644 --- a/activesupport/lib/active_support/security_utils.rb +++ b/activesupport/lib/active_support/security_utils.rb @@ -2,14 +2,12 @@ require "digest" module ActiveSupport module SecurityUtils - # Constant time string comparison. + # Constant time string comparison, for fixed length strings. # # The values compared should be of fixed length, such as strings - # that have already been processed by HMAC. This should not be used - # on variable length plaintext strings because it could leak length info - # via timing attacks. - def secure_compare(a, b) - return false unless a.bytesize == b.bytesize + # that have already been processed by HMAC. Raises in case of length mismatch. + def fixed_length_secure_compare(a, b) + raise ArgumentError, "string length mismatch." unless a.bytesize == b.bytesize l = a.unpack "C#{a.bytesize}" @@ -17,11 +15,15 @@ module ActiveSupport b.each_byte { |byte| res |= byte ^ l.shift } res == 0 end - module_function :secure_compare + module_function :fixed_length_secure_compare - def variable_size_secure_compare(a, b) # :nodoc: - secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b)) + # Constant time string comparison, for variable length strings. + # + # The values are first processed by SHA256, so that we don't leak length info + # via timing attacks. + def secure_compare(a, b) + fixed_length_secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b)) end - module_function :variable_size_secure_compare + module_function :secure_compare end end -- cgit v1.2.3