Skip to content

Commit

Permalink
Use secure_compare for hmac comparison
Browse files Browse the repository at this point in the history 
 * Closes CVE-2013-0263
  • Loading branch information
@raggi
raggi committed Feb 7, 2013
1 parent 9a81b96 commit 0cd7e9a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/rack/session/cookie.rb
View file
Expand Up @@ -159,7 +159,7 @@ def destroy_session(env, session_id, options)
def digest_match?(data, digest) def digest_match?(data, digest)
return unless data && digest return unless data && digest
@secrets.any? do |secret| @secrets.any? do |secret|
digest == generate_hmac(data, secret) Rack::Utils.secure_compare(digest, generate_hmac(data, secret))

This comment has been minimized.

Sign in to view

Copy link
@tarcieri

tarcieri Feb 8, 2013

Heh, funny, I looked for this vulnerability in Rails recently and saw they were using a (wonky) constant-time comparison function. Didn't think to look in Rack. Crypto is hard :(

This comment has been minimized.

Sign in to view

Copy link
@dchapman1988

dchapman1988 Feb 8, 2013

More support for RbNaCl 👍

This comment has been minimized.

Sign in to view

Copy link
@tarcieri

tarcieri Feb 8, 2013

Constant time comparison function? Yeah, RbNaCl's got one of those (and full HMAC as implemented by Dan Bernstein): https://github.com/cryptosphere/rbnacl/blob/master/lib/rbnacl/util.rb#L40

This comment has been minimized.

Sign in to view

Copy link
@raggi

raggi Feb 8, 2013

Author Member

Saddest thing about this is, @codahale reported this 3 years ago, and I even responded then, but I was too dumb to get it, and not running releases (probably good). Anyway, I was wrong then, and we were wrong not to deal with it.

This comment has been minimized.

Sign in to view

Copy link
@tarcieri

tarcieri Feb 8, 2013

@raggi again, crypto is hard ;(

This comment has been minimized.

Sign in to view

Copy link
@raggi

raggi Feb 8, 2013

Author Member

@tarcieri thing that really gets me though, is that these days I'm very clearly aware of how critical timing attacks are. I had some generally knowledgable folks telling me it's not really viable over the last couple of days. Well, when you're inside the Cloud, you're basically on a LAN. This is totally viable inside [insert cloud service here].

This comment has been minimized.

Sign in to view

Copy link
@tarcieri

tarcieri Feb 8, 2013

Know a timing attack:

Screen Shot 2013-02-07 at 10 06 03 PM
end end
end end


Expand Down

0 comments on commit 0cd7e9a

Please sign in to comment.