Skip to content

Commit ae19e41

Browse files
NZKoztenderlove
authored andcommitted
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 0b58a7f commit ae19e41

File tree

3 files changed

+86
-147
lines changed

3 files changed

+86
-147
lines changed

actionpack/lib/action_controller/metal/request_forgery_protection.rb

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,25 +71,24 @@ def protect_from_forgery(options = {})
7171
end
7272

7373
protected
74-
75-
def protect_from_forgery(options = {})
76-
self.request_forgery_protection_token ||= :authenticity_token
77-
before_filter :verify_authenticity_token, options
78-
end
79-
8074
# The actual before_filter that is used. Modify this to change how you handle unverified requests.
8175
def verify_authenticity_token
82-
verified_request? || raise(ActionController::InvalidAuthenticityToken)
76+
verified_request? || handle_unverified_request
77+
end
78+
79+
def handle_unverified_request
80+
reset_session
8381
end
8482

8583
# Returns true or false if a request is verified. Checks:
8684
#
87-
# * is the format restricted? By default, only HTML requests are checked.
8885
# * is it a GET request? Gets should be safe and idempotent
8986
# * Does the form_authenticity_token match the given token value from the params?
87+
# * Does the X-CSRF-Token header match the form_authenticity_token
9088
def verified_request?
91-
!protect_against_forgery? || request.forgery_whitelisted? ||
92-
form_authenticity_token == params[request_forgery_protection_token]
89+
!protect_against_forgery? || request.get? ||
90+
form_authenticity_token == params[request_forgery_protection_token] ||
91+
form_authenticity_token == request.headers['X-CSRF-Token']
9392
end
9493

9594
# Sets the token value for the current session.

actionpack/lib/action_dispatch/http/request.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,9 @@ def fullpath
133133
end
134134

135135
def forgery_whitelisted?
136-
get? || xhr? || content_mime_type.nil? || !content_mime_type.verify_request?
136+
get?
137137
end
138+
deprecate :forgery_whitelisted? => "it is just an alias for 'get?' now, update your code"
138139

139140
def media_type
140141
content_mime_type.to_s

actionpack/test/controller/request_forgery_protection_test.rb

Lines changed: 75 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ class RequestForgeryProtectionController < ActionController::Base
4545
protect_from_forgery :only => %w(index meta)
4646
end
4747

48+
class RequestForgeryProtectionControllerUsingOldBehaviour < ActionController::Base
49+
include RequestForgeryProtectionActions
50+
protect_from_forgery :only => %w(index meta)
51+
52+
def handle_unverified_request
53+
raise(ActionController::InvalidAuthenticityToken)
54+
end
55+
end
56+
57+
4858
class FreeCookieController < RequestForgeryProtectionController
4959
self.allow_forgery_protection = false
5060

@@ -67,172 +77,92 @@ def form_authenticity_param
6777
# common test methods
6878

6979
module RequestForgeryProtectionTests
70-
def teardown
71-
ActionController::Base.request_forgery_protection_token = nil
72-
end
80+
def setup
81+
@token = "cf50faa3fe97702ca1ae"
7382

74-
def test_should_render_form_with_token_tag
75-
get :index
76-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
83+
ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
84+
ActionController::Base.request_forgery_protection_token = :authenticity_token
7785
end
7886

79-
def test_should_render_external_form_for_with_external_token
80-
get :external_form_for
81-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', 'external_token'
82-
end
8387

84-
def test_should_render_form_for_without_token_tag
85-
get :form_for_without_protection
86-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false
88+
def test_should_render_form_with_token_tag
89+
assert_not_blocked do
90+
get :index
91+
end
92+
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
8793
end
8894

8995
def test_should_render_button_to_with_token_tag
90-
get :show_button
96+
assert_not_blocked do
97+
get :show_button
98+
end
9199
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
92100
end
93101

94-
def test_should_render_external_form_with_external_token
95-
get :external_form
96-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', 'external_token'
97-
end
98-
99-
def test_should_render_external_form_without_token
100-
get :external_form_without_protection
101-
assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token, false
102-
end
103-
104102
def test_should_allow_get
105-
get :index
106-
assert_response :success
103+
assert_not_blocked { get :index }
107104
end
108105

109106
def test_should_allow_post_without_token_on_unsafe_action
110-
post :unsafe
111-
assert_response :success
112-
end
113-
114-
def test_should_not_allow_html_post_without_token
115-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
116-
assert_raise(ActionController::InvalidAuthenticityToken) { post :index, :format => :html }
117-
end
118-
119-
def test_should_not_allow_html_put_without_token
120-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
121-
assert_raise(ActionController::InvalidAuthenticityToken) { put :index, :format => :html }
122-
end
123-
124-
def test_should_not_allow_html_delete_without_token
125-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
126-
assert_raise(ActionController::InvalidAuthenticityToken) { delete :index, :format => :html }
127-
end
128-
129-
def test_should_allow_api_formatted_post_without_token
130-
assert_nothing_raised do
131-
post :index, :format => 'xml'
132-
end
133-
end
134-
135-
def test_should_not_allow_api_formatted_put_without_token
136-
assert_nothing_raised do
137-
put :index, :format => 'xml'
138-
end
139-
end
140-
141-
def test_should_allow_api_formatted_delete_without_token
142-
assert_nothing_raised do
143-
delete :index, :format => 'xml'
144-
end
145-
end
146-
147-
def test_should_not_allow_api_formatted_post_sent_as_url_encoded_form_without_token
148-
assert_raise(ActionController::InvalidAuthenticityToken) do
149-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
150-
post :index, :format => 'xml'
151-
end
152-
end
153-
154-
def test_should_not_allow_api_formatted_put_sent_as_url_encoded_form_without_token
155-
assert_raise(ActionController::InvalidAuthenticityToken) do
156-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
157-
put :index, :format => 'xml'
158-
end
159-
end
160-
161-
def test_should_not_allow_api_formatted_delete_sent_as_url_encoded_form_without_token
162-
assert_raise(ActionController::InvalidAuthenticityToken) do
163-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
164-
delete :index, :format => 'xml'
165-
end
107+
assert_not_blocked { post :unsafe }
166108
end
167109

168-
def test_should_not_allow_api_formatted_post_sent_as_multipart_form_without_token
169-
assert_raise(ActionController::InvalidAuthenticityToken) do
170-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
171-
post :index, :format => 'xml'
172-
end
110+
def test_should_not_allow_post_without_token
111+
assert_blocked { post :index }
173112
end
174113

175-
def test_should_not_allow_api_formatted_put_sent_as_multipart_form_without_token
176-
assert_raise(ActionController::InvalidAuthenticityToken) do
177-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
178-
put :index, :format => 'xml'
179-
end
114+
def test_should_not_allow_post_without_token_irrespective_of_format
115+
assert_blocked { post :index, :format=>'xml' }
180116
end
181117

182-
def test_should_not_allow_api_formatted_delete_sent_as_multipart_form_without_token
183-
assert_raise(ActionController::InvalidAuthenticityToken) do
184-
@request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
185-
delete :index, :format => 'xml'
186-
end
118+
def test_should_not_allow_put_without_token
119+
assert_blocked { put :index }
187120
end
188121

189-
def test_should_allow_xhr_post_without_token
190-
assert_nothing_raised { xhr :post, :index }
122+
def test_should_not_allow_delete_without_token
123+
assert_blocked { delete :index }
191124
end
192125

193-
def test_should_allow_xhr_put_without_token
194-
assert_nothing_raised { xhr :put, :index }
126+
def test_should_not_allow_xhr_post_without_token
127+
assert_blocked { xhr :post, :index }
195128
end
196129

197-
def test_should_allow_xhr_delete_without_token
198-
assert_nothing_raised { xhr :delete, :index }
130+
def test_should_allow_post_with_token
131+
assert_not_blocked { post :index, :authenticity_token => @token }
199132
end
200133

201-
def test_should_allow_xhr_post_with_encoded_form_content_type_without_token
202-
@request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
203-
assert_nothing_raised { xhr :post, :index }
134+
def test_should_allow_put_with_token
135+
assert_not_blocked { put :index, :authenticity_token => @token }
204136
end
205137

206-
def test_should_allow_post_with_token
207-
post :index, :authenticity_token => @token
208-
assert_response :success
138+
def test_should_allow_delete_with_token
139+
assert_not_blocked { delete :index, :authenticity_token => @token }
209140
end
210141

211-
def test_should_allow_put_with_token
212-
put :index, :authenticity_token => @token
213-
assert_response :success
142+
def test_should_allow_post_with_token_in_header
143+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
144+
assert_not_blocked { post :index }
214145
end
215146

216-
def test_should_allow_delete_with_token
217-
delete :index, :authenticity_token => @token
218-
assert_response :success
147+
def test_should_allow_delete_with_token_in_header
148+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
149+
assert_not_blocked { delete :index }
219150
end
220151

221-
def test_should_allow_post_with_xml
222-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
223-
post :index, :format => 'xml'
224-
assert_response :success
152+
def test_should_allow_put_with_token_in_header
153+
@request.env['HTTP_X_CSRF_TOKEN'] = @token
154+
assert_not_blocked { put :index }
225155
end
226156

227-
def test_should_allow_put_with_xml
228-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
229-
put :index, :format => 'xml'
157+
def assert_blocked
158+
session[:something_like_user_id] = 1
159+
yield
160+
assert_nil session[:something_like_user_id], "session values are still present"
230161
assert_response :success
231162
end
232163

233-
def test_should_allow_delete_with_xml
234-
@request.env['CONTENT_TYPE'] = Mime::XML.to_s
235-
delete :index, :format => 'xml'
164+
def assert_not_blocked
165+
assert_nothing_raised { yield }
236166
assert_response :success
237167
end
238168
end
@@ -241,16 +171,6 @@ def test_should_allow_delete_with_xml
241171

242172
class RequestForgeryProtectionControllerTest < ActionController::TestCase
243173
include RequestForgeryProtectionTests
244-
def setup
245-
@controller = RequestForgeryProtectionController.new
246-
@request = ActionController::TestRequest.new
247-
@request.format = :html
248-
@response = ActionController::TestResponse.new
249-
@token = "cf50faa3fe97702ca1ae"
250-
251-
ActiveSupport::SecureRandom.stubs(:base64).returns(@token)
252-
ActionController::Base.request_forgery_protection_token = :authenticity_token
253-
end
254174

255175
test 'should emit a csrf-token meta tag' do
256176
ActiveSupport::SecureRandom.stubs(:base64).returns(@token + '<=?')
@@ -262,6 +182,15 @@ def setup
262182
end
263183
end
264184

185+
class RequestForgeryProtectionControllerUsingOldBehaviourTest < ActionController::TestCase
186+
include RequestForgeryProtectionTests
187+
def assert_blocked
188+
assert_raises(ActionController::InvalidAuthenticityToken) do
189+
yield
190+
end
191+
end
192+
end
193+
265194
class FreeCookieControllerTest < ActionController::TestCase
266195
def setup
267196
@controller = FreeCookieController.new
@@ -294,13 +223,23 @@ def test_should_allow_all_methods_without_token
294223
end
295224
end
296225

226+
227+
228+
229+
297230
class CustomAuthenticityParamControllerTest < ActionController::TestCase
298231
def setup
232+
ActionController::Base.request_forgery_protection_token = :custom_token_name
233+
super
234+
end
235+
236+
def teardown
299237
ActionController::Base.request_forgery_protection_token = :authenticity_token
238+
super
300239
end
301240

302241
def test_should_allow_custom_token
303-
post :index, :authenticity_token => 'foobar'
242+
post :index, :custom_token_name => 'foobar'
304243
assert_response :ok
305244
end
306245
end

0 commit comments

Comments
 (0)