Skip to content

Commit

Permalink
Change the CSRF whitelisting to only apply to get requests
Browse files Browse the repository at this point in the history 
Unfortunately the previous method of browser detection and XHR whitelisting is unable to prevent requests issued from some Flash animations and Java applets.  To ease the work required to include the CSRF token in ajax requests rails now supports providing the token in a custom http header:

 X-CSRF-Token: ...

This fixes CVE-2011-0447
  • Loading branch information
@NZKoz
NZKoz committed Jan 17, 2011
1 parent b4a0d1b commit b5d759f
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 128 deletions.
20 changes: 14 additions & 6 deletions actionpack/lib/action_controller/request_forgery_protection.rb
View file
Expand Up @@ -83,7 +83,11 @@ def protect_from_forgery(options = {})
protected protected
# The actual before_filter that is used. Modify this to change how you handle unverified requests. # The actual before_filter that is used. Modify this to change how you handle unverified requests.
def verify_authenticity_token def verify_authenticity_token
verified_request? || raise(ActionController::InvalidAuthenticityToken) verified_request? || handle_unverified_request
end

def handle_unverified_request
reset_session
end end


# Returns true or false if a request is verified. Checks: # Returns true or false if a request is verified. Checks:
Expand All @@ -92,12 +96,16 @@ def verify_authenticity_token
# * is it a GET request? Gets should be safe and idempotent # * is it a GET request? Gets should be safe and idempotent
# * Does the form_authenticity_token match the given _token value from the params? # * Does the form_authenticity_token match the given _token value from the params?
def verified_request? def verified_request?
!protect_against_forgery? || !protect_against_forgery? ||
request.method == :get || request.get? ||
!verifiable_request_format? || form_authenticity_token == form_authenticity_param ||
form_authenticity_token == params[request_forgery_protection_token] form_authenticity_token == request.headers['X-CSRF-Token']
end end


def form_authenticity_param
params[request_forgery_protection_token]
end

def verifiable_request_format? def verifiable_request_format?
request.content_type.nil? || request.content_type.verify_request? request.content_type.nil? || request.content_type.verify_request?
end end
Expand Down
14 changes: 14 additions & 0 deletions actionpack/lib/action_view/helpers/csrf_helper.rb
View file
@@ -0,0 +1,14 @@
module ActionView
# = Action View CSRF Helper
module Helpers
module CsrfHelper
# Returns a meta tag with the cross-site request forgery protection token
# for forms to use. Place this in your head.
def csrf_meta_tag
if protect_against_forgery?
%(<meta name="csrf-param" content="#{h(request_forgery_protection_token)}"/>\n<meta name="csrf-token" content="#{h(form_authenticity_token)}"/>)
end
end
end
end
end
217 changes: 95 additions & 122 deletions actionpack/test/controller/request_forgery_protection_test.rb
View file
Expand Up @@ -30,6 +30,10 @@ def unsafe
render :text => 'pwn' render :text => 'pwn'
end end


def meta
render :inline => "<%= csrf_meta_tag %>"
end

def rescue_action(e) raise e end def rescue_action(e) raise e end
end end


Expand Down Expand Up @@ -58,7 +62,17 @@ def rescue_action(e) raise e end
include RequestForgeryProtectionActions include RequestForgeryProtectionActions
end end


class FreeCookieController < CsrfCookieMonsterController class RequestForgeryProtectionControllerUsingOldBehaviour < ActionController::Base
include RequestForgeryProtectionActions
protect_from_forgery :only => %w(index meta)

def handle_unverified_request
raise(ActionController::InvalidAuthenticityToken)
end
end


class FreeCookieController < RequestForgeryProtectionController
self.allow_forgery_protection = false self.allow_forgery_protection = false


def index def index
Expand All @@ -78,144 +92,109 @@ def teardown
end end


def test_should_render_form_with_token_tag def test_should_render_form_with_token_tag
get :index get :index
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
end end

def test_should_render_button_to_with_token_tag
get :show_button
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
end

def test_should_render_remote_form_with_only_one_token_parameter
get :remote_form
assert_equal 1, @response.body.scan(@token).size
end

def test_should_allow_get
get :index
assert_response :success
end

def test_should_allow_post_without_token_on_unsafe_action
post :unsafe
assert_response :success
end



def test_should_render_button_to_with_token_tag def test_should_render_form_with_token_tag
get :show_button assert_not_blocked do
get :index
end
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
end end


def test_should_render_remote_form_with_only_one_token_parameter def test_should_render_button_to_with_token_tag
get :remote_form assert_not_blocked do
assert_equal 1, @response.body.scan(@token).size get :show_button
end
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
end end


def test_should_allow_get def test_should_allow_get
get :index assert_not_blocked { get :index }
assert_response :success
end end

def test_should_allow_post_without_token_on_unsafe_action def test_should_allow_post_without_token_on_unsafe_action
post :unsafe assert_not_blocked { post :unsafe }
assert_response :success
end end


def test_should_not_allow_post_without_token def test_should_not_allow_post_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { post :index } assert_blocked { post :index }
end

def test_should_not_allow_post_without_token_irrespective_of_format
assert_blocked { post :index, :format=>'xml' }
end end


def test_should_not_allow_put_without_token def test_should_not_allow_put_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { put :index } assert_blocked { put :index }
end end


def test_should_not_allow_delete_without_token def test_should_not_allow_delete_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { delete :index } assert_blocked { delete :index }
end

def test_should_not_allow_api_formatted_post_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
post :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_put_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
put :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_delete_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
delete :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_post_sent_as_url_encoded_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
post :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_put_sent_as_url_encoded_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
put :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_delete_sent_as_url_encoded_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
delete :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_post_sent_as_multipart_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
post :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_put_sent_as_multipart_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
put :index, :format => 'xml'
end
end

def test_should_not_allow_api_formatted_delete_sent_as_multipart_form_without_token
assert_raises(ActionController::InvalidAuthenticityToken) do
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
delete :index, :format => 'xml'
end
end end


def test_should_not_allow_xhr_post_without_token def test_should_not_allow_xhr_post_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { xhr :post, :index } assert_blocked { xhr :post, :index }
end

def test_should_not_allow_xhr_put_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { xhr :put, :index }
end

def test_should_not_allow_xhr_delete_without_token
assert_raises(ActionController::InvalidAuthenticityToken) { xhr :delete, :index }
end end

def test_should_allow_post_with_token def test_should_allow_post_with_token
post :index, :authenticity_token => @token assert_not_blocked { post :index, :authenticity_token => @token }
assert_response :success
end end


def test_should_allow_put_with_token def test_should_allow_put_with_token
put :index, :authenticity_token => @token assert_not_blocked { put :index, :authenticity_token => @token }
assert_response :success
end end


def test_should_allow_delete_with_token def test_should_allow_delete_with_token
delete :index, :authenticity_token => @token assert_not_blocked { delete :index, :authenticity_token => @token }
assert_response :success
end end


def test_should_allow_post_with_xml def test_should_allow_post_with_token_in_header
@request.env['CONTENT_TYPE'] = Mime::XML.to_s @request.env['HTTP_X_CSRF_TOKEN'] = @token
post :index, :format => 'xml' assert_not_blocked { post :index }
assert_response :success end

def test_should_allow_delete_with_token_in_header
@request.env['HTTP_X_CSRF_TOKEN'] = @token
assert_not_blocked { delete :index }
end end


def test_should_allow_put_with_xml def test_should_allow_put_with_token_in_header
@request.env['CONTENT_TYPE'] = Mime::XML.to_s @request.env['HTTP_X_CSRF_TOKEN'] = @token
put :index, :format => 'xml' assert_not_blocked { put :index }
end

def assert_blocked
@request.session[:something_like_user_id] = 1
yield
assert_nil @request.session[:something_like_user_id], "session values are still present"
assert_response :success assert_response :success
end end


def test_should_allow_delete_with_xml def assert_not_blocked
@request.env['CONTENT_TYPE'] = Mime::XML.to_s assert_nothing_raised { yield }
delete :index, :format => 'xml'
assert_response :success assert_response :success
end end
end end
Expand All @@ -234,6 +213,11 @@ def session_id() '123' end
@token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123') @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
ActionController::Base.request_forgery_protection_token = :authenticity_token ActionController::Base.request_forgery_protection_token = :authenticity_token
end end

def test_should_emit_meta_tag
get :meta
assert_equal %(<meta name="csrf-param" content="authenticity_token"/>\n<meta name="csrf-token" content="#{@token}"/>), @response.body
end
end end


class RequestForgeryProtectionWithoutSecretControllerTest < Test::Unit::TestCase class RequestForgeryProtectionWithoutSecretControllerTest < Test::Unit::TestCase
Expand All @@ -248,11 +232,11 @@ def session_id() '123' end
ActionController::Base.request_forgery_protection_token = :authenticity_token ActionController::Base.request_forgery_protection_token = :authenticity_token
end end


def test_should_raise_error_without_secret def test_should_raise_error_without_secret
assert_raises ActionController::InvalidAuthenticityToken do assert_raises ActionController::InvalidAuthenticityToken do
get :index get :index
end end
end end
end end


class CsrfCookieMonsterControllerTest < Test::Unit::TestCase class CsrfCookieMonsterControllerTest < Test::Unit::TestCase
Expand Down Expand Up @@ -294,20 +278,9 @@ def test_should_allow_all_methods_without_token
assert_nothing_raised { send(method, :index)} assert_nothing_raised { send(method, :index)}
end end
end end
end


class SessionOffControllerTest < Test::Unit::TestCase def test_should_not_emit_meta_tag
def setup get :meta
@controller = SessionOffController.new assert @response.body.blank?, "Response body should be blank"
@request = ActionController::TestRequest.new
@response = ActionController::TestResponse.new
@token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
end

def test_should_raise_correct_exception
@request.session = {} # session(:off) doesn't appear to work with controller tests
assert_raises(ActionController::InvalidAuthenticityToken) do
post :index, :authenticity_token => @token
end
end end
end end

0 comments on commit b5d759f

Please sign in to comment.