aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAaron Patterson <aaron.patterson@gmail.com>2015-10-29 10:42:44 -0700
committerAaron Patterson <aaron.patterson@gmail.com>2016-01-22 14:57:14 -0800
commita6fa3960c3a149e83eb2ff057be4472a82958e3d (patch)
tree5f1fec107a443ee54507b9001edfcee8483d551c
parent9dc8ddc39424818a3d713a353353ac20cb431218 (diff)
downloadrails-a6fa3960c3a149e83eb2ff057be4472a82958e3d.tar.gz
rails-a6fa3960c3a149e83eb2ff057be4472a82958e3d.tar.bz2
rails-a6fa3960c3a149e83eb2ff057be4472a82958e3d.zip
use secure string comparisons for basic auth username / password
this will avoid timing attacks against applications that use basic auth. Conflicts: activesupport/lib/active_support/security_utils.rb Conflicts: actionpack/lib/action_controller/metal/http_authentication.rb CVE-2015-7576
-rw-r--r--actionpack/lib/action_controller/metal/http_authentication.rb7
-rw-r--r--activesupport/lib/active_support/security_utils.rb27
2 files changed, 33 insertions, 1 deletions
diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb
index fe4ab65bba..2ae516097a 100644
--- a/actionpack/lib/action_controller/metal/http_authentication.rb
+++ b/actionpack/lib/action_controller/metal/http_authentication.rb
@@ -1,5 +1,6 @@
require 'active_support/base64'
require 'active_support/core_ext/object/blank'
+require 'active_support/security_utils'
module ActionController
module HttpAuthentication
@@ -111,7 +112,11 @@ module ActionController
def http_basic_authenticate_with(options = {})
before_filter(options.except(:name, :password, :realm)) do
authenticate_or_request_with_http_basic(options[:realm] || "Application") do |name, password|
- name == options[:name] && password == options[:password]
+ # This comparison uses & so that it doesn't short circuit and
+ # uses `variable_size_secure_compare` so that length information
+ # isn't leaked.
+ ActiveSupport::SecurityUtils.variable_size_secure_compare(name, options[:name]) &
+ ActiveSupport::SecurityUtils.variable_size_secure_compare(password, options[:password])
end
end
end
diff --git a/activesupport/lib/active_support/security_utils.rb b/activesupport/lib/active_support/security_utils.rb
new file mode 100644
index 0000000000..9be8613ada
--- /dev/null
+++ b/activesupport/lib/active_support/security_utils.rb
@@ -0,0 +1,27 @@
+require 'digest'
+
+module ActiveSupport
+ module SecurityUtils
+ # Constant time string comparison.
+ #
+ # 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
+
+ l = a.unpack "C#{a.bytesize}"
+
+ res = 0
+ b.each_byte { |byte| res |= byte ^ l.shift }
+ res == 0
+ end
+ module_function :secure_compare
+
+ def variable_size_secure_compare(a, b) # :nodoc:
+ secure_compare(::Digest::SHA256.hexdigest(a), ::Digest::SHA256.hexdigest(b))
+ end
+ module_function :variable_size_secure_compare
+ end
+end