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 -
http://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection/ClassMethods.html

  • 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

On Apr 1, 1:52 am, JSW [email protected] wrote:

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?

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

On Thu, Apr 1, 2010 at 1:08 AM, Frederick C.
<[email protected]

wrote:

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.

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

On Thu, Apr 1, 2010 at 7:20 AM, Jeff L. [email protected] wrote:

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.

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