Skip to content

Commit 7e86f9b

Browse files
committed
Change the CSRF whitelisting to only apply to get requests
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
1 parent abe9773 commit 7e86f9b

File tree

4 files changed

+117
-130
lines changed

4 files changed

+117
-130
lines changed

actionpack/lib/action_controller/request_forgery_protection.rb

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,11 @@ def protect_from_forgery(options = {})
7676
protected
7777
# The actual before_filter that is used. Modify this to change how you handle unverified requests.
7878
def verify_authenticity_token
79-
verified_request? || raise(ActionController::InvalidAuthenticityToken)
79+
verified_request? || handle_unverified_request
80+
end
81+
82+
def handle_unverified_request
83+
reset_session
8084
end
8185

8286
# Returns true or false if a request is verified. Checks:
@@ -85,11 +89,10 @@ def verify_authenticity_token
8589
# * is it a GET request? Gets should be safe and idempotent
8690
# * Does the form_authenticity_token match the given token value from the params?
8791
def verified_request?
88-
!protect_against_forgery? ||
89-
request.method == :get ||
90-
request.xhr? ||
91-
!verifiable_request_format? ||
92-
form_authenticity_token == form_authenticity_param
92+
!protect_against_forgery? ||
93+
request.get? ||
94+
form_authenticity_token == form_authenticity_param ||
95+
form_authenticity_token == request.headers['X-CSRF-Token']
9396
end
9497

9598
def form_authenticity_param

actionpack/lib/action_view/helpers.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module Helpers #:nodoc:
66
autoload :BenchmarkHelper, 'action_view/helpers/benchmark_helper'
77
autoload :CacheHelper, 'action_view/helpers/cache_helper'
88
autoload :CaptureHelper, 'action_view/helpers/capture_helper'
9+
autoload :CsrfHelper, 'action_view/helpers/csrf_helper'
910
autoload :DateHelper, 'action_view/helpers/date_helper'
1011
autoload :DebugHelper, 'action_view/helpers/debug_helper'
1112
autoload :FormHelper, 'action_view/helpers/form_helper'
@@ -38,6 +39,7 @@ module ClassMethods
3839
include BenchmarkHelper
3940
include CacheHelper
4041
include CaptureHelper
42+
include CsrfHelper
4143
include DateHelper
4244
include DebugHelper
4345
include FormHelper
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
module ActionView
2+
# = Action View CSRF Helper
3+
module Helpers
4+
module CsrfHelper
5+
# Returns a meta tag with the cross-site request forgery protection token
6+
# for forms to use. Place this in your head.
7+
def csrf_meta_tag
8+
if protect_against_forgery?
9+
%(<meta name="csrf-param" content="#{h(request_forgery_protection_token)}"/>\n<meta name="csrf-token" content="#{h(form_authenticity_token)}"/>).html_safe
10+
end
11+
end
12+
end
13+
end
14+
end

actionpack/test/controller/request_forgery_protection_test.rb

Lines changed: 92 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ def unsafe
2323
render :text => 'pwn'
2424
end
2525

26+
def meta
27+
render :inline => "<%= csrf_meta_tag %>"
28+
end
29+
2630
def rescue_action(e) raise e end
2731
end
2832

@@ -32,6 +36,16 @@ class RequestForgeryProtectionController < ActionController::Base
3236
protect_from_forgery :only => :index
3337
end
3438

39+
class RequestForgeryProtectionControllerUsingOldBehaviour < ActionController::Base
40+
include RequestForgeryProtectionActions
41+
protect_from_forgery :only => %w(index meta)
42+
43+
def handle_unverified_request
44+
raise(ActionController::InvalidAuthenticityToken)
45+
end
46+
end
47+
48+
3549
class FreeCookieController < RequestForgeryProtectionController
3650
self.allow_forgery_protection = false
3751

@@ -54,158 +68,92 @@ def form_authenticity_param
5468
# common test methods
5569

5670
module RequestForgeryProtectionTests
57-
def teardown
58-
ActionController::Base.request_forgery_protection_token = nil
59-
end
60-
71+
def setup
72+
@token = "cf50faa3fe97702ca1ae"
6173

62-
def test_should_render_form_with_token_tag
63-
get :index
64-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
65-
end
66-
67-
def test_should_render_button_to_with_token_tag
68-
get :show_button
69-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
70-
end
71-
72-
def test_should_render_remote_form_with_only_one_token_parameter
73-
get :remote_form
74-
assert_equal 1, @response.body.scan(@token).size
75-
end
76-
77-
def test_should_allow_get
78-
get :index
79-
assert_response :success
80-
end
81-
82-
def test_should_allow_post_without_token_on_unsafe_action
83-
post :unsafe
84-
assert_response :success
85-
end
86-
87-
def test_should_not_allow_html_post_without_token
88-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
89-
assert_raise(ActionController::InvalidAuthenticityToken) { post :index, :format => :html }
74+
ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
75+
ActionController::Base.request_forgery_protection_token = :authenticity_token
9076
end
9177

92-
def test_should_not_allow_html_put_without_token
93-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
94-
assert_raise(ActionController::InvalidAuthenticityToken) { put :index, :format => :html }
95-
end
9678

97-
def test_should_not_allow_html_delete_without_token
98-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
99-
assert_raise(ActionController::InvalidAuthenticityToken) { delete :index, :format => :html }
100-
end
101-
102-
def test_should_allow_api_formatted_post_without_token
103-
assert_nothing_raised do
104-
post :index, :format => 'xml'
79+
def test_should_render_form_with_token_tag
80+
assert_not_blocked do
81+
get :index
10582
end
83+
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
10684
end
10785

108-
def test_should_not_allow_api_formatted_put_without_token
109-
assert_nothing_raised do
110-
put :index, :format => 'xml'
86+
def test_should_render_button_to_with_token_tag
87+
assert_not_blocked do
88+
get :show_button
11189
end
90+
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
11291
end
11392

114-
def test_should_allow_api_formatted_delete_without_token
115-
assert_nothing_raised do
116-
delete :index, :format => 'xml'
117-
end
93+
def test_should_allow_get
94+
assert_not_blocked { get :index }
11895
end
11996

120-
def test_should_not_allow_api_formatted_post_sent_as_url_encoded_form_without_token
121-
assert_raise(ActionController::InvalidAuthenticityToken) do
122-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
123-
post :index, :format => 'xml'
124-
end
97+
def test_should_allow_post_without_token_on_unsafe_action
98+
assert_not_blocked { post :unsafe }
12599
end
126100

127-
def test_should_not_allow_api_formatted_put_sent_as_url_encoded_form_without_token
128-
assert_raise(ActionController::InvalidAuthenticityToken) do
129-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
130-
put :index, :format => 'xml'
131-
end
101+
def test_should_not_allow_post_without_token
102+
assert_blocked { post :index }
132103
end
133104

134-
def test_should_not_allow_api_formatted_delete_sent_as_url_encoded_form_without_token
135-
assert_raise(ActionController::InvalidAuthenticityToken) do
136-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
137-
delete :index, :format => 'xml'
138-
end
105+
def test_should_not_allow_post_without_token_irrespective_of_format
106+
assert_blocked { post :index, :format=>'xml' }
139107
end
140108

141-
def test_should_not_allow_api_formatted_post_sent_as_multipart_form_without_token
142-
assert_raise(ActionController::InvalidAuthenticityToken) do
143-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
144-
post :index, :format => 'xml'
145-
end
109+
def test_should_not_allow_put_without_token
110+
assert_blocked { put :index }
146111
end
147112

148-
def test_should_not_allow_api_formatted_put_sent_as_multipart_form_without_token
149-
assert_raise(ActionController::InvalidAuthenticityToken) do
150-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
151-
put :index, :format => 'xml'
152-
end
113+
def test_should_not_allow_delete_without_token
114+
assert_blocked { delete :index }
153115
end
154116

155-
def test_should_not_allow_api_formatted_delete_sent_as_multipart_form_without_token
156-
assert_raise(ActionController::InvalidAuthenticityToken) do
157-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
158-
delete :index, :format => 'xml'
159-
end
160-
end
161-
162-
def test_should_allow_xhr_post_without_token
163-
assert_nothing_raised { xhr :post, :index }
164-
end
165-
166-
def test_should_allow_xhr_put_without_token
167-
assert_nothing_raised { xhr :put, :index }
168-
end
169-
170-
def test_should_allow_xhr_delete_without_token
171-
assert_nothing_raised { xhr :delete, :index }
117+
def test_should_not_allow_xhr_post_without_token
118+
assert_blocked { xhr :post, :index }
172119
end
173-
174-
def test_should_allow_xhr_post_with_encoded_form_content_type_without_token
175-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
176-
assert_nothing_raised { xhr :post, :index }
177-
end
178-
120+
179121
def test_should_allow_post_with_token
180-
post :index, :authenticity_token => @token
181-
assert_response :success
122+
assert_not_blocked { post :index, :authenticity_token => @token }
182123
end
183124

184125
def test_should_allow_put_with_token
185-
put :index, :authenticity_token => @token
186-
assert_response :success
126+
assert_not_blocked { put :index, :authenticity_token => @token }
187127
end
188128

189129
def test_should_allow_delete_with_token
190-
delete :index, :authenticity_token => @token
191-
assert_response :success
130+
assert_not_blocked { delete :index, :authenticity_token => @token }
192131
end
193132

194-
def test_should_allow_post_with_xml
195-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
196-
post :index, :format => 'xml'
197-
assert_response :success
133+
def test_should_allow_post_with_token_in_header
134+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
135+
assert_not_blocked { post :index }
136+
end
137+
138+
def test_should_allow_delete_with_token_in_header
139+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
140+
assert_not_blocked { delete :index }
198141
end
199142

200-
def test_should_allow_put_with_xml
201-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
202-
put :index, :format => 'xml'
143+
def test_should_allow_put_with_token_in_header
144+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
145+
assert_not_blocked { put :index }
146+
end
147+
148+
def assert_blocked
149+
session[:something_like_user_id] = 1
150+
yield
151+
assert_nil session[:something_like_user_id], "session values are still present"
203152
assert_response :success
204153
end
205154

206-
def test_should_allow_delete_with_xml
207-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
208-
delete :index, :format => 'xml'
155+
def assert_not_blocked
156+
assert_nothing_raised { yield }
209157
assert_response :success
210158
end
211159
end
@@ -214,15 +162,20 @@ def test_should_allow_delete_with_xml
214162

215163
class RequestForgeryProtectionControllerTest < ActionController::TestCase
216164
include RequestForgeryProtectionTests
217-
def setup
218-
@controller = RequestForgeryProtectionController.new
219-
@request = ActionController::TestRequest.new
220-
@request.format = :html
221-
@response = ActionController::TestResponse.new
222-
@token = "cf50faa3fe97702ca1ae"
223165

224-
ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
225-
ActionController::Base.request_forgery_protection_token = :authenticity_token
166+
test 'should emit a csrf-token meta tag' do
167+
ActiveSupport::SecureRandom.stubs(:base64).returns(@token + '<=?')
168+
get :meta
169+
assert_equal %(<meta name="csrf-param" content="authenticity_token"/>\n<meta name="csrf-token" content="cf50faa3fe97702ca1ae&lt;=?"/>), @response.body
170+
end
171+
end
172+
173+
class RequestForgeryProtectionControllerUsingOldBehaviourTest < ActionController::TestCase
174+
include RequestForgeryProtectionTests
175+
def assert_blocked
176+
assert_raises(ActionController::InvalidAuthenticityToken) do
177+
yield
178+
end
226179
end
227180
end
228181

@@ -251,15 +204,30 @@ def test_should_allow_all_methods_without_token
251204
assert_nothing_raised { send(method, :index)}
252205
end
253206
end
207+
208+
test 'should not emit a csrf-token meta tag' do
209+
get :meta
210+
assert_blank @response.body
211+
end
254212
end
255213

214+
215+
216+
217+
256218
class CustomAuthenticityParamControllerTest < ActionController::TestCase
257219
def setup
220+
ActionController::Base.request_forgery_protection_token = :custom_token_name
221+
super
222+
end
223+
224+
def teardown
258225
ActionController::Base.request_forgery_protection_token = :authenticity_token
226+
super
259227
end
260228

261229
def test_should_allow_custom_token
262-
post :index, :authenticity_token => 'foobar'
230+
post :index, :custom_token_name => 'foobar'
263231
assert_response :ok
264232
end
265233
end

0 commit comments

Comments
 (0)