Skip to content
This repository
Browse code

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
  • Loading branch information...
commit c6cb5a5ab00ac9e857e5b2757d2bce6a5ad14b32 1 parent 785281a
Michael Koziarski authored January 13, 2011
20  actionpack/lib/action_controller/request_forgery_protection.rb
@@ -83,7 +83,11 @@ def protect_from_forgery(options = {})
83 83
     protected
84 84
       # The actual before_filter that is used.  Modify this to change how you handle unverified requests.
85 85
       def verify_authenticity_token
86  
-        verified_request? || raise(ActionController::InvalidAuthenticityToken)
  86
+        verified_request? || handle_unverified_request
  87
+      end
  88
+
  89
+      def handle_unverified_request
  90
+        reset_session
87 91
       end
88 92
       
89 93
       # Returns true or false if a request is verified.  Checks:
@@ -92,12 +96,16 @@ def verify_authenticity_token
92 96
       # * is it a GET request?  Gets should be safe and idempotent
93 97
       # * Does the form_authenticity_token match the given _token value from the params?
94 98
       def verified_request?
95  
-        !protect_against_forgery?     ||
96  
-          request.method == :get      ||
97  
-          !verifiable_request_format? ||
98  
-          form_authenticity_token == params[request_forgery_protection_token]
  99
+        !protect_against_forgery?                            ||
  100
+          request.get?                                       ||
  101
+          form_authenticity_token == form_authenticity_param ||
  102
+          form_authenticity_token == request.headers['X-CSRF-Token']
99 103
       end
100  
-    
  104
+
  105
+      def form_authenticity_param
  106
+        params[request_forgery_protection_token]
  107
+      end
  108
+
101 109
       def verifiable_request_format?
102 110
         !request.content_type.nil? && request.content_type.verify_request?
103 111
       end
1  actionpack/lib/action_view/helpers.rb
@@ -19,6 +19,7 @@ module ClassMethods
19 19
     include BenchmarkHelper
20 20
     include CacheHelper
21 21
     include CaptureHelper
  22
+    include CsrfHelper
22 23
     include DateHelper
23 24
     include DebugHelper
24 25
     include FormHelper
14  actionpack/lib/action_view/helpers/csrf_helper.rb
... ...
@@ -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)}"/>)
  10
+        end
  11
+      end
  12
+    end
  13
+  end
  14
+end
185  actionpack/test/controller/request_forgery_protection_test.rb
@@ -30,6 +30,10 @@ def unsafe
30 30
     render :text => 'pwn'
31 31
   end
32 32
   
  33
+  def meta
  34
+    render :inline => "<%= csrf_meta_tag %>"
  35
+  end
  36
+
33 37
   def rescue_action(e) raise e end
34 38
 end
35 39
 
@@ -58,7 +62,17 @@ def rescue_action(e) raise e end
58 62
   include RequestForgeryProtectionActions
59 63
 end
60 64
 
61  
-class FreeCookieController < CsrfCookieMonsterController
  65
+class RequestForgeryProtectionControllerUsingOldBehaviour < ActionController::Base
  66
+  include RequestForgeryProtectionActions
  67
+  protect_from_forgery :only => %w(index meta)
  68
+
  69
+  def handle_unverified_request
  70
+    raise(ActionController::InvalidAuthenticityToken)
  71
+  end
  72
+end
  73
+
  74
+
  75
+class FreeCookieController < RequestForgeryProtectionController
62 76
   self.allow_forgery_protection = false
63 77
   
64 78
   def index
@@ -103,127 +117,85 @@ def test_should_allow_post_without_token_on_unsafe_action
103 117
      assert_response :success
104 118
    end
105 119
 
106  
-  def test_should_not_allow_html_post_without_token
107  
-    @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
108  
-    assert_raises(ActionController::InvalidAuthenticityToken) { post :index, :format => :html }
109  
-  end
110 120
   
111  
-  def test_should_not_allow_html_put_without_token
112  
-    @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
113  
-    assert_raises(ActionController::InvalidAuthenticityToken) { put :index, :format => :html }
114  
-  end
115  
-  
116  
-  def test_should_not_allow_html_delete_without_token
117  
-    @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
118  
-    assert_raises(ActionController::InvalidAuthenticityToken) { delete :index, :format => :html }
119  
-  end
120  
-
121  
-  def test_should_allow_api_formatted_post_without_token
122  
-    assert_nothing_raised do
123  
-      post :index, :format => 'xml'
  121
+  def test_should_render_form_with_token_tag
  122
+    assert_not_blocked do
  123
+      get :index
124 124
     end
  125
+    assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
125 126
   end
126 127
 
127  
-  def test_should_not_allow_api_formatted_put_without_token
128  
-    assert_nothing_raised do
129  
-      put :index, :format => 'xml'
  128
+  def test_should_render_button_to_with_token_tag
  129
+    assert_not_blocked do
  130
+      get :show_button
130 131
     end
  132
+    assert_select 'form>div>input[name=?][value=?]', 'authenticity_token', @token
131 133
   end
132 134
 
133  
-  def test_should_allow_api_formatted_delete_without_token
134  
-    assert_nothing_raised do
135  
-      delete :index, :format => 'xml'
136  
-    end
  135
+  def test_should_allow_get
  136
+    assert_not_blocked { get :index }
137 137
   end
138 138
 
139  
-  def test_should_not_allow_api_formatted_post_sent_as_url_encoded_form_without_token
140  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
141  
-      @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
142  
-      post :index, :format => 'xml'
143  
-    end
  139
+  def test_should_allow_post_without_token_on_unsafe_action
  140
+    assert_not_blocked { post :unsafe }
144 141
   end
145 142
 
146  
-  def test_should_not_allow_api_formatted_put_sent_as_url_encoded_form_without_token
147  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
148  
-      @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
149  
-      put :index, :format => 'xml'
150  
-    end
  143
+  def test_should_not_allow_post_without_token
  144
+    assert_blocked { post :index }
151 145
   end
152  
-
153  
-  def test_should_not_allow_api_formatted_delete_sent_as_url_encoded_form_without_token
154  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
155  
-      @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
156  
-      delete :index, :format => 'xml'
157  
-    end
  146
+  
  147
+  def test_should_not_allow_post_without_token_irrespective_of_format
  148
+    assert_blocked { post :index, :format=>'xml' }
158 149
   end
159 150
 
160  
-  def test_should_not_allow_api_formatted_post_sent_as_multipart_form_without_token
161  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
162  
-      @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
163  
-      post :index, :format => 'xml'
164  
-    end
  151
+  def test_should_not_allow_put_without_token
  152
+    assert_blocked { put :index }
165 153
   end
166 154
 
167  
-  def test_should_not_allow_api_formatted_put_sent_as_multipart_form_without_token
168  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
169  
-      @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
170  
-      put :index, :format => 'xml'
171  
-    end
  155
+  def test_should_not_allow_delete_without_token
  156
+    assert_blocked { delete :index }
172 157
   end
173 158
 
174  
-  def test_should_not_allow_api_formatted_delete_sent_as_multipart_form_without_token
175  
-    assert_raises(ActionController::InvalidAuthenticityToken) do
176  
-      @request.env['CONTENT_TYPE'] = Mime::MULTIPART_FORM.to_s
177  
-      delete :index, :format => 'xml'
178  
-    end
  159
+  def test_should_not_allow_xhr_post_without_token
  160
+    assert_blocked { xhr :post, :index }
179 161
   end
180 162
 
181  
-  def test_should_allow_xhr_post_without_token
182  
-    assert_nothing_raised { xhr :post, :index }
183  
-  end
184  
-  def test_should_not_allow_xhr_post_with_html_without_token
185  
-    @request.env['CONTENT_TYPE'] = Mime::URL_ENCODED_FORM.to_s
186  
-    assert_raise(ActionController::InvalidAuthenticityToken) { xhr :post, :index }
187  
-  end
188  
-  
189  
-  def test_should_allow_xhr_put_without_token
190  
-    assert_nothing_raised { xhr :put, :index }
191  
-  end
192  
-  
193  
-  def test_should_allow_xhr_delete_without_token
194  
-    assert_nothing_raised { xhr :delete, :index }
195  
-  end
196  
-  
197 163
   def test_should_allow_post_with_token
198  
-    post :index, :authenticity_token => @token
199  
-    assert_response :success
  164
+    assert_not_blocked { post :index, :authenticity_token => @token }
200 165
   end
201 166
   
202 167
   def test_should_allow_put_with_token
203  
-    put :index, :authenticity_token => @token
204  
-    assert_response :success
  168
+    assert_not_blocked { put :index, :authenticity_token => @token }
205 169
   end
206 170
   
207 171
   def test_should_allow_delete_with_token
208  
-    delete :index, :authenticity_token => @token
209  
-    assert_response :success
  172
+    assert_not_blocked { delete :index, :authenticity_token => @token }
210 173
   end
211 174
   
212  
-  def test_should_allow_post_with_xml
213  
-    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
214  
-    post :index, :format => 'xml'
215  
-    assert_response :success
  175
+  def test_should_allow_post_with_token_in_header
  176
+    @request.env['HTTP_X_CSRF_TOKEN'] = @token
  177
+    assert_not_blocked { post :index }
  178
+  end
  179
+
  180
+  def test_should_allow_delete_with_token_in_header
  181
+    @request.env['HTTP_X_CSRF_TOKEN'] = @token
  182
+    assert_not_blocked { delete :index }
216 183
   end
217 184
   
218  
-  def test_should_allow_put_with_xml
219  
-    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
220  
-    put :index, :format => 'xml'
  185
+  def test_should_allow_put_with_token_in_header
  186
+    @request.env['HTTP_X_CSRF_TOKEN'] = @token
  187
+    assert_not_blocked { put :index }
  188
+  end
  189
+    
  190
+  def assert_blocked
  191
+    @request.session[:something_like_user_id] = 1
  192
+    yield
  193
+    assert_nil @request.session[:something_like_user_id], "session values are still present"
221 194
     assert_response :success
222 195
   end
223 196
   
224  
-  def test_should_allow_delete_with_xml
225  
-    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
226  
-    delete :index, :format => 'xml'
  197
+  def assert_not_blocked
  198
+    assert_nothing_raised { yield }
227 199
     assert_response :success
228 200
   end
229 201
 end
@@ -243,6 +215,11 @@ def session_id() '123' end
243 215
     @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
244 216
     ActionController::Base.request_forgery_protection_token = :authenticity_token
245 217
   end
  218
+
  219
+  def test_should_emit_meta_tag 
  220
+    get :meta
  221
+    assert_equal %(<meta name="csrf-param" content="authenticity_token"/>\n<meta name="csrf-token" content="#{@token}"/>), @response.body
  222
+  end
246 223
 end
247 224
 
248 225
 class RequestForgeryProtectionWithoutSecretControllerTest < Test::Unit::TestCase
@@ -257,11 +234,11 @@ def session_id() '123' end
257 234
     ActionController::Base.request_forgery_protection_token = :authenticity_token
258 235
   end
259 236
   
260  
-  # def test_should_raise_error_without_secret
261  
-  #   assert_raises ActionController::InvalidAuthenticityToken do
262  
-  #     get :index
263  
-  #   end
264  
-  # end
  237
+   def test_should_raise_error_without_secret
  238
+     assert_raises ActionController::InvalidAuthenticityToken do
  239
+       get :index
  240
+     end
  241
+   end
265 242
 end
266 243
 
267 244
 class CsrfCookieMonsterControllerTest < Test::Unit::TestCase
@@ -303,25 +280,9 @@ def test_should_allow_all_methods_without_token
303 280
       assert_nothing_raised { send(method, :index)}
304 281
     end
305 282
   end
306  
-end
307 283
 
308  
-class SessionOffControllerTest < Test::Unit::TestCase
309  
-  def setup
310  
-    @controller = SessionOffController.new
311  
-    @request    = ActionController::TestRequest.new
312  
-    @response   = ActionController::TestResponse.new
313  
-    @token      = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123')
  284
+  def test_should_not_emit_meta_tag 
  285
+    get :meta
  286
+    assert @response.body.blank?, "Response body should be blank"
314 287
   end
315  
-
316  
-  # TODO: Rewrite this test.
317  
-  # This test was passing but for the wrong reason.
318  
-  # Sessions aren't really being turned off, so an exception was raised
319  
-  # because sessions weren't on - not because the token didn't match.
320  
-  #
321  
-  # def test_should_raise_correct_exception
322  
-  #   @request.session = {} # session(:off) doesn't appear to work with controller tests
323  
-  #   assert_raises(ActionController::InvalidAuthenticityToken) do
324  
-  #     post :index, :authenticity_token => @token, :format => :html
325  
-  #   end
326  
-  # end
327 288
 end

0 notes on commit c6cb5a5

Please sign in to comment.
Something went wrong with that request. Please try again.