Change the request forgery protection to go by Content-Type instead of request.format so that you can't bypass it by POSTing to "#{request.uri}.xml" [#73 state:resolved]

3 files changed, 28 insertions, 5 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 54030047ba..87f570d55c 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Change the request forgery protection to go by Content-Type instead of request.format so that you can't bypass it by POSTing to "#{request.uri}.xml" [rick]
+
 * Fixed that TextHelper#text_field would corrypt when raw HTML was used as the value (mchenryc, Kevin Glowacz) [#80]
 
 * Added ActionController::TestCase#rescue_action_in_public! to control whether the action under test should use the regular rescue_action path instead of simply raising the exception inline (great for error testing) [DHH]
diff --git a/actionpack/lib/action_controller/request_forgery_protection.rb b/actionpack/lib/action_controller/request_forgery_protection.rb
index 7e6961d25f..1b349baeaf 100644
--- a/actionpack/lib/action_controller/request_forgery_protection.rb
+++ b/actionpack/lib/action_controller/request_forgery_protection.rb
@@ -99,7 +99,7 @@ module ActionController #:nodoc:
       end
     
       def verifiable_request_format?
-        request.format.html? || request.format.js?
+        request.content_type.nil? || request.content_type.html?
       end
     
       # Sets the token value for the current session.  Pass a <tt>:secret</tt> option
diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb
index d0c3c6e224..295075fed4 100644
--- a/actionpack/test/controller/request_forgery_protection_test.rb
+++ b/actionpack/test/controller/request_forgery_protection_test.rb
@@ -93,19 +93,37 @@ module RequestForgeryProtectionTests
     post :unsafe
     assert_response :success
   end
-  
+
   def test_should_not_allow_post_without_token
     assert_raises(ActionController::InvalidAuthenticityToken) { post :index }
   end
-  
+
   def test_should_not_allow_put_without_token
     assert_raises(ActionController::InvalidAuthenticityToken) { put :index }
   end
-  
+
   def test_should_not_allow_delete_without_token
     assert_raises(ActionController::InvalidAuthenticityToken) { 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_put_without_token
+    assert_raises(ActionController::InvalidAuthenticityToken) do
+      put :index, :format => 'xml'
+    end
+  end
+
+  def test_should_not_allow_delete_without_token
+    assert_raises(ActionController::InvalidAuthenticityToken) do
+      delete :index, :format => 'xml'
+    end
+  end
+
   def test_should_not_allow_xhr_post_without_token
     assert_raises(ActionController::InvalidAuthenticityToken) { xhr :post, :index }
   end
@@ -134,16 +152,19 @@ module RequestForgeryProtectionTests
   end
   
   def test_should_allow_post_with_xml
+    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
     post :index, :format => 'xml'
     assert_response :success
   end
   
   def test_should_allow_put_with_xml
+    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
     put :index, :format => 'xml'
     assert_response :success
   end
   
   def test_should_allow_delete_with_xml
+    @request.env['CONTENT_TYPE'] = Mime::XML.to_s
     delete :index, :format => 'xml'
     assert_response :success
   end