mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
HMAC raw CSRF token before masking it, so it cannot be used to reconstruct a per-form token
[CVE-2020-8166]
This commit is contained in:
parent
9b4aef4be3
commit
358ff18975
2 changed files with 60 additions and 7 deletions
|
@ -322,13 +322,10 @@ module ActionController #:nodoc:
|
|||
action_path = normalize_action_path(action)
|
||||
per_form_csrf_token(session, action_path, method)
|
||||
else
|
||||
real_csrf_token(session)
|
||||
global_csrf_token(session)
|
||||
end
|
||||
|
||||
one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
|
||||
encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token)
|
||||
masked_token = one_time_pad + encrypted_csrf_token
|
||||
Base64.urlsafe_encode64(masked_token, padding: false)
|
||||
mask_token(raw_token)
|
||||
end
|
||||
|
||||
# Checks the client's masked token to see if it matches the
|
||||
|
@ -358,7 +355,8 @@ module ActionController #:nodoc:
|
|||
elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2
|
||||
csrf_token = unmask_token(masked_token)
|
||||
|
||||
compare_with_real_token(csrf_token, session) ||
|
||||
compare_with_global_token(csrf_token, session) ||
|
||||
compare_with_real_token(csrf_token, session) ||
|
||||
valid_per_form_csrf_token?(csrf_token, session)
|
||||
else
|
||||
false # Token is malformed.
|
||||
|
@ -373,10 +371,21 @@ module ActionController #:nodoc:
|
|||
xor_byte_strings(one_time_pad, encrypted_csrf_token)
|
||||
end
|
||||
|
||||
def mask_token(raw_token) # :doc:
|
||||
one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH)
|
||||
encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token)
|
||||
masked_token = one_time_pad + encrypted_csrf_token
|
||||
Base64.urlsafe_encode64(masked_token, padding: false)
|
||||
end
|
||||
|
||||
def compare_with_real_token(token, session) # :doc:
|
||||
ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session))
|
||||
end
|
||||
|
||||
def compare_with_global_token(token, session) # :doc:
|
||||
ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, global_csrf_token(session))
|
||||
end
|
||||
|
||||
def valid_per_form_csrf_token?(token, session) # :doc:
|
||||
if per_form_csrf_tokens
|
||||
correct_token = per_form_csrf_token(
|
||||
|
@ -397,10 +406,21 @@ module ActionController #:nodoc:
|
|||
end
|
||||
|
||||
def per_form_csrf_token(session, action_path, method) # :doc:
|
||||
csrf_token_hmac(session, [action_path, method.downcase].join("#"))
|
||||
end
|
||||
|
||||
GLOBAL_CSRF_TOKEN_IDENTIFIER = "!real_csrf_token"
|
||||
private_constant :GLOBAL_CSRF_TOKEN_IDENTIFIER
|
||||
|
||||
def global_csrf_token(session) # :doc:
|
||||
csrf_token_hmac(session, GLOBAL_CSRF_TOKEN_IDENTIFIER)
|
||||
end
|
||||
|
||||
def csrf_token_hmac(session, identifier) # :doc:
|
||||
OpenSSL::HMAC.digest(
|
||||
OpenSSL::Digest::SHA256.new,
|
||||
real_csrf_token(session),
|
||||
[action_path, method.downcase].join("#")
|
||||
identifier
|
||||
)
|
||||
end
|
||||
|
||||
|
|
|
@ -953,6 +953,39 @@ class PerFormTokensControllerTest < ActionController::TestCase
|
|||
assert_response :success
|
||||
end
|
||||
|
||||
def test_does_not_return_old_csrf_token
|
||||
get :index
|
||||
|
||||
token = @controller.send(:form_authenticity_token)
|
||||
|
||||
unmasked_token = @controller.send(:unmask_token, Base64.urlsafe_decode64(token))
|
||||
|
||||
assert_not_equal @controller.send(:real_csrf_token, session), unmasked_token
|
||||
end
|
||||
|
||||
def test_returns_hmacd_token
|
||||
get :index
|
||||
|
||||
token = @controller.send(:form_authenticity_token)
|
||||
|
||||
unmasked_token = @controller.send(:unmask_token, Base64.urlsafe_decode64(token))
|
||||
|
||||
assert_equal @controller.send(:global_csrf_token, session), unmasked_token
|
||||
end
|
||||
|
||||
def test_accepts_old_csrf_token
|
||||
get :index
|
||||
|
||||
non_hmac_token = @controller.send(:mask_token, @controller.send(:real_csrf_token, session))
|
||||
|
||||
# This is required because PATH_INFO isn't reset between requests.
|
||||
@request.env["PATH_INFO"] = "/per_form_tokens/post_one"
|
||||
assert_nothing_raised do
|
||||
post :post_one, params: { custom_authenticity_token: non_hmac_token }
|
||||
end
|
||||
assert_response :success
|
||||
end
|
||||
|
||||
def test_chomps_slashes
|
||||
get :index, params: { form_path: "/per_form_tokens/post_one?foo=bar" }
|
||||
|
||||
|
|
Loading…
Reference in a new issue