POST-only logic in protect_from_forgery considered harmful?

Hi folks,

I am just getting into rails again after a multi-year stint of mod_perl jobs, which might grant me some newbie-indemnity for the time being - but I've found an issue I think warrants discussion.

As discussed here - ActionController::RequestForgeryProtection::ClassMethods - the CSRF protection feature does not kick in for GET requests. This is under the assumption that GET requests are idempotent.

There is a (big, IMO) problem with this: unless the controller action which receives the POST request manually validates that the request is a POST as expected, it is wide open to CSRF. All the attacker has to do is construct the same form submission that would be a POST as an unexpected GET request instead, which is actually easier to do from a remote CSRF attackers point of view. The controller will then happily do all the work requested of it without the token being present at all.

How many rails developers do we think put a POST-method validation filter around all their form processing code, and yet expect protect_from_forgery stills somehow protects the actions?

Also, it appears that the authenticity token is not inserted into forms automatically when the form is specified as a GET. While GET requests *should* be idempotent, it is quite common for them not to be. Consider for instance on Yahoo! search which remembers (server- side) your most recent searches. Searches then are effectively non- idempotent GET requests and should include a CSRF protection as a result.

So in reality, GET vs POST is much more of an HTTP distinction than an application distinction, and framework-level "blessed" security measures should not hinge on adherence to protocol practices that are easily and often pragmatically bent or broken, especially when it's up to the attacker which method to invoke.

IMO this feature is providing a rather false sense of security to the community without documenting the clear steps needed to take in order to actually obtain CSRF protection for a particular action.

JSW

Without getting into the debate about how idempotent GET requests really are I'd suspect that these days most people are using restful routes. If you use restful routes and remove the default route then it's not possible invoke (eg) a create action from a get request.

Fred

A fair point. Though certainly the use of restful routing is optional and even if used in most cases, I’d wager it is common to find important code happening outside it. In those cases, we have a fail-open scenario with the current filter.

The hard part is coming up with a fail-closed filter that will catch these but isn’t also a pain to manage for normal requests. At the moment, I cannot think of an elegant solution, this is just one of those things where developer diligence is the solution to security. I’m just concerned the framework-blessed blanket solution is creating a lurking risk in the ecosystem of rails apps.

On a related note, does anyone know if rails 3 is taking a different approach to this issue?

jsw

This seems like a non-issue to me that can and should be handled by the developer of the app, regardless of what lang/framework you're using, by following basic best-practices for securing your app against csrf or sql-injection or ... attack.

So in your post example, if you didn't want to restrict some controller method based on a route rule (ie ... map.connect ... :conditions=>{:method=>:post} ...), then all you'd have to do is add a simple check in your controller meth, like:

  ...   if request.post?     # do post-related work ...   end   ...

or alternatively:

  ...   if not request.post?     # log, redirect with err msg ...   end   ...

Bottom line is that an app is only as secure as the developer makes it.

Jeff

I let the topic fallow for a bit to see if anyone else wanted to chime in; but I’m honestly a little concerned that nobody else is concerned about this one.

I would argue that the “basic best practices” for securing your app against CSRF are not entirely trivial, and I think the existence of the protect_from_forgery feature itself is testimony to that. When a framework platform (Rails or any other) makes an attempt at a security protection, it needs to do it carefully so as not to create a false sense of security. It should either (a) genuinely take care of it for you, (b) pass on the issue entirely making it 100% your problem (like how rails 2.x passes on XSS), or (b) provide help but still make sure you’re fully aware it’s not a complete solution. The current implementation does none of those. Consequently, when I recently took over an app from someone else I found unexpectedly that it was vulnerable to CSRF due to this hinging of the logic on the HTTP verb, which is really not an essential aspect of a CSRF attack.

In any case, I hope that raising this on-list helps others in my situation to locate their own unexpected CSRF vulnerabilites. Please be sure to validate your actions as POST if you expect them to benefit from protect_from_forgery.

That’s all, thanks :slight_smile:

jsw