-
-
Notifications
You must be signed in to change notification settings - Fork 302
Description
Description
OmniAuth::Strategies::OAuth2#secure_compare crashes with NoMethodError: undefined method 'bytesize' for nil when the session state has been lost (expired session, cleared cookies, etc.) between the initial authorization redirect and the OAuth2 callback.
Expected behavior
When session.delete("omniauth.state") returns nil, the CSRF state check should fail gracefully and call fail!(:csrf_detected, ...), the same as it does when the state parameter is empty.
Actual behavior
secure_compare is called with nil as the second argument and crashes:
NoMethodError: undefined method 'bytesize' for nil
OmniAuth's top-level rescue StandardError catches this and redirects to the failure endpoint with the raw exception message, which is confusing for users and bypasses any strategy-specific error handling.
Steps to reproduce
- Start an OAuth2 authorization flow (user is redirected to the provider)
- Before the callback completes, clear the session (e.g., session expires, cookies cleared, or user opens callback URL in a different browser)
- The callback URL still has a valid
stateparameter, butsession["omniauth.state"]is nownil
Root cause
In lib/omniauth/strategies/oauth2.rb:
def callback_phase
error = request.params["error_reason"] || request.params["error"]
if !options.provider_ignores_state && (request.params["state"].to_s.empty? || !secure_compare(request.params["state"], session.delete("omniauth.state")))
fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))The guard request.params["state"].to_s.empty? only checks if the request param is empty. When it's present but the session state is nil, execution reaches secure_compare(valid_string, nil).
In secure_compare:
def secure_compare(string_a, string_b)
return false unless string_a.bytesize == string_b.bytesize # 💥 nil.bytesizeSuggested fix
Add a nil guard to secure_compare:
def secure_compare(string_a, string_b)
return false if string_a.nil? || string_b.nil?
return false unless string_a.bytesize == string_b.bytesize
# ...
endThis makes secure_compare return false for nil inputs, which correctly falls through to fail!(:csrf_detected, ...) — the intended behavior for a missing/mismatched state.
Versions
omniauth-oauth21.9.0omniauth2.1.4- Ruby 3.4.1