Skip to content
Permalink
Browse files Browse the repository at this point in the history
Use secure_compare when checking CSRF token
Since string comparisions may return early we want to use a constant
time comparsion function to protect the CSRF token against timing
attacks. Rack::Utils provides a such function.
  • Loading branch information
jeltz authored and zzak committed Jul 26, 2016
1 parent c419868 commit 8aa6c42
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
4 changes: 2 additions & 2 deletions rack-protection/lib/rack/protection/authenticity_token.rb
Expand Up @@ -23,8 +23,8 @@ def accepts?(env)
session = session env
token = session[:csrf] ||= session['_csrf_token'] || random_string
safe?(env) ||
env['HTTP_X_CSRF_TOKEN'] == token ||
Request.new(env).params[options[:authenticity_param]] == token
secure_compare(env['HTTP_X_CSRF_TOKEN'], token) ||
secure_compare(Request.new(env).params[options[:authenticity_param]], token)
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions rack-protection/lib/rack/protection/base.rb
@@ -1,4 +1,5 @@
require 'rack/protection'
require 'rack/utils'
require 'digest'
require 'logger'
require 'uri'
Expand Down Expand Up @@ -110,6 +111,10 @@ def encrypt(value)
options[:encryptor].hexdigest value.to_s
end

def secure_compare(a, b)
Rack::Utils.secure_compare(a.to_s, b.to_s)
end

alias default_reaction deny

def html?(headers)
Expand Down

3 comments on commit 8aa6c42

@zzak
Copy link
Member

@zzak zzak commented on 8aa6c42 Mar 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kseifriedredhat! I don't think we ever did, here is the original report: sinatra/rack-protection#98

We merged rack-protection into sinatra source when releasing 2.0, which includes this patch. Prior versions to 2.0 shouldn't have it.

@namusyaka
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. Unfortunately, rack-1.5.x is still being used yet, so I think the commit should be backported. I'm going to work on that this weekend.

@namusyaka
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kseifriedredhat Thank you. Just backported in https://rubygems.org/gems/rack-protection/versions/1.5.5

Please sign in to comment.