Skip to content

Commit d9f3243

Browse files
committed
Prevent timing attack on CSRF, completing wonderful pr by @eutopian
1 parent 3e7ee11 commit d9f3243

File tree

2 files changed

+22
-2
lines changed

2 files changed

+22
-2
lines changed

Diff for: lib/omniauth/strategies/oauth2.rb

+12-2
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,7 @@ def token_params
8383

8484
def callback_phase # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
8585
error = request.params["error_reason"] || request.params["error"]
86-
if !options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))
87-
fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))
86+
elsif !options.provider_ignores_state && (request.params["state"].to_s.empty? || !secure_compare(request.params["state"], session.delete("omniauth.state")))
8887
elsif error
8988
fail!(error, CallbackError.new(request.params["error"], request.params["error_description"] || request.params["error_reason"], request.params["error_uri"]))
9089
else
@@ -144,6 +143,17 @@ def options_for(option)
144143
hash
145144
end
146145

146+
# constant-time comparison algorithm to prevent timing attacks
147+
def secure_compare(string_a, string_b)
148+
return false unless string_a.bytesize == string_b.bytesize
149+
150+
l = string_a.unpack "C#{string_a.bytesize}"
151+
152+
res = 0
153+
string_b.each_byte { |byte| res |= byte ^ l.shift }
154+
res.zero?
155+
end
156+
147157
# An error that is indicated in the OAuth 2.0 callback.
148158
# This could be a `redirect_uri_mismatch` or other
149159
class CallbackError < StandardError

Diff for: spec/omniauth/strategies/oauth2_spec.rb

+10
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,16 @@ def app
168168
end
169169
end
170170
end
171+
172+
describe "#secure_compare" do
173+
subject { fresh_strategy }
174+
175+
it "returns true when the two inputs are the same and false otherwise" do
176+
instance = subject.new("abc", "def")
177+
expect(instance.send(:secure_compare, "a", "a")).to be true
178+
expect(instance.send(:secure_compare, "b", "a")).to be false
179+
end
180+
end
171181
end
172182

173183
describe OmniAuth::Strategies::OAuth2::CallbackError do

0 commit comments

Comments
 (0)