Skip to content

Commit 938b858

Browse files
committed
use secure compare instead of equal compare to prevent timing attacks
1 parent 01fbfb5 commit 938b858

File tree

1 file changed

+12
-1
lines changed

1 file changed

+12
-1
lines changed

lib/omniauth/strategies/oauth2.rb

+12-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def callback_phase # rubocop:disable AbcSize, CyclomaticComplexity, MethodLength
6767
error = request.params["error_reason"] || request.params["error"]
6868
if error
6969
fail!(error, CallbackError.new(request.params["error"], request.params["error_description"] || request.params["error_reason"], request.params["error_uri"]))
70-
elsif !options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))
70+
elsif !options.provider_ignores_state && (request.params["state"].to_s.empty? || !secure_compare(request.params["state"], session.delete("omniauth.state")))
7171
fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))
7272
else
7373
self.access_token = build_access_token
@@ -105,6 +105,17 @@ def options_for(option)
105105
hash
106106
end
107107

108+
# constant-time comparison algorithm to prevent timing attacks
109+
def secure_compare(a, b)
110+
return false unless a.bytesize == b.bytesize
111+
112+
l = a.unpack "C#{a.bytesize}"
113+
114+
res = 0
115+
b.each_byte { |byte| res |= byte ^ l.shift }
116+
res == 0
117+
end
118+
108119
# An error that is indicated in the OAuth 2.0 callback.
109120
# This could be a `redirect_uri_mismatch` or other
110121
class CallbackError < StandardError

0 commit comments

Comments
 (0)