Rails Ruby on Rails

Comments and changes to this ticket

• 

  Frederick Cheung May 1st, 2008 @ 10:43 AM

  As i see it this is only a problem if you implicitly trust json input more than a 'normal' http post, and i can't think why you would do that (and indeed ajax posts also need to include the auth token).

• You flagged this item as spam.
  

  Mathijs Kwik May 1st, 2008 @ 11:57 AM

  That's not what I'm reporting here.

  The thing is, that in the case I mentioned (submitting a form from a different domain to a url ending in .json or .xml) bypasses the RF-protection completely!

  So this is a security-bug, since the feature doesn't work as 'advertised'.

  What you are talking about is extending RF-protection to include other kinds of requests.

  I think this really isn't needed, since RF-protection was only meant to protect against forms being submitted from different domains.

  At the moment, ajax posts are protected automatically because of the same-origin policy, so it's not possible for an attacker to set-up a site that sends ajax-requests to your domain.

  This is why XML and JSON requests aren't checked by RF-protection.

  To include RF-protection in xml/json requests would mean a lot of stuff (including ActiveResource) would have to be patched.

  I personally don't think it is needed to extend RF-protection in these areas, but like I said, this is a different subject.

  My report was only about RF-protection not working in a case that it should work.

  So this is an exploitable bug to bypass a security-feature.

  My solution would be to check for the Content-Type header instead of the Accept header.

• 

  DHH May 4th, 2008 @ 04:52 AM

  • Assigned user set to “Rick”
• 

  Rick May 6th, 2008 @ 09:33 AM

  • no changes were found...

  • 0001-Change-the-request-forgery-protection-to-go-by-Conte.patch 3.9 KB
• 

  Rick May 6th, 2008 @ 09:34 AM

  Here's my commit with the change. It's in my personal fork until I've tested it in Lighthouse and deployed it. I'd appreciate if anyone else using CSRF protection in rails would do the same. This makes sense to use content-type since the REST routes for the POST/PUT/DELETE methods check the content type too.

• 

  Rick May 6th, 2008 @ 10:59 AM

  Ah, forgot about Mime::MULTIPART_FORM and Mime::URL_ENCODED_FORM. Wee.

• 

  Rick May 6th, 2008 @ 11:08 AM

  If you want to protect your Rails 2.0 app:

  class ApplicationController < ActionController::Base
    def verifiable_request_format?
      request.content_type != Mime::JSON && request.content_type != Mime::XML
    end
  end
  

  You want to check any request that isn't JSON or XML. Though to be honest it doesn't matter much since you can tweak headers with AJAX or Flash. Tokens should be checked on every request, or sessions should also be disabled on non-html requests.

  class ApplicationController < ActionController::Base
    session :off, 
      :if => Proc.new { |req| !(req.format.html? || req.format.js?) }
  end
  

• You flagged this item as spam.
  

  Mathijs Kwik May 6th, 2008 @ 12:45 PM

  Rick, Thanks for the patch,

  I will look into it at the end of the day.

  One thing about turning off the session and about protecting ajax-requests in general...

  I use a lot of json to communicate between my front-end application(ExtJS) and my rails-resources. I wish to use the normal session to authenticate these requests, since sending user/pass on every request would be insecure on HTTP (logging in is HTTPS). Furthermore, since all this still takes place in the web-browser, it feels natural to have 1 session for all communication.

  Is there a danger if I don't disable the session for json/xml requests as you say?

  All non-ajax (simple form) requests from any domain will be blocked by CSRF because of their content-type now.

  Like you said, you can tweak headers with AJAX.

  But to the best of my knowledge, AJAX is protected by the same-origin-policy, so I think protecting ajax-requests isn't needed anyway, since there is no way for an attacker to forge an ajax-request(using javascript on his own website) that talks with my server. Even if the content-type is html.

  What about Flash?

  I know flash isn't subject to the same-origin-policy, but is it possible for flash to use a session the browser might have with my server?

  Any thoughts about this are very welcome.

  I'll let you know if the patch works for me.

• 

  Repository May 13th, 2008 @ 05:46 PM

  (from [c8451aeeea200043d8a3e6eae9c49def3a154ddb]) change ActionController::RequestForgeryProtection to use Mime::Type#verify_request? [#73]

  http://github.com/rails/rails/co...

• 

  Repository May 13th, 2008 @ 05:46 PM

  • State changed from “new” to “resolved”

  (from [0697d17d121fcf9f46b5dd2dd1034dffa19ebdf2]) 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]

  http://github.com/rails/rails/co...

• 

  Rick May 13th, 2008 @ 05:55 PM

  I've been testing these out on Lighthouse for the past week without problems. I think we're doing better now, but please shower us with patches/failing test cases/complaints...

• You flagged this item as spam.
  

  Matthew Moore September 30th, 2008 @ 08:51 PM

  • Tag set to “request-forgery-protection, actionpack, security”

  This actually causes problems for XML delete requests, especially when using ActiveResource.

  The problem stems from the assumption that an XML delete request will have an XML content-type. A correct XML Delete request shouldn't have a content payload at all. Instead, it should just have the correct format (xml), and correct HTTP_ACCEPTs set.

  This test isn't right:

  
     def test_should_allow_delete_with_xml
      @request.env['CONTENT_TYPE'] = Mime::XML.to_s
       delete :index, :format => 'xml'
       assert_response :success
     end
  

  If we're assuming a "correct" delete request via XML, the test would look more like the following, which currently fails:

  
     def test_should_allow_delete_with_xml
      @request.env['HTTP_ACCEPT'] = Mime::XML.to_s
       delete :index, :format => 'xml'
       assert_response :success
     end
  

  I've put up a ticket here: Bug: InvalidAuthenticityToken incorrectly raised for XML controller#destroy request

• You flagged this item as spam.
  

  Mathijs Kwik October 1st, 2008 @ 07:10 AM

  I responded to your post at http://rails.lighthouseapp.com/p...

  I don't think this ticket should be re-opened, let's discuss the issue there.