Whitelist Your Routes, "match" is Evil | Hacker News Reader

Whitelist Your Routes, "match" is Evil

homakov.blogspot.com

76 pointshomakov45 comments

Proposal to make "match" deprecated because it is useless and insecure.

This reads like one of those ransom notes from the movies where each letter is cut out of a different magazine, so I don't quite get what he's saying.

Instead of writing:

    do_thing()

He's suggesting you write:

    if(preconditions_are_met())
        do_thing()

Sounds good to me.

in a nutshell - something like this.

I propose ALWAYS specify HTTP verb for action - because you always know which way controller#action should be used. If you dont - you are doing it wrong.

Cool. You might want to use less bold text, BTW :)

Haha :) +1 I have very poor vocabulary - and use bold to emphasize important points. Thanks for pointing it out - I will extend my vocab.

Russian speaker here - moved to US when I was ten, haven't had much opportunity to practice. Your English is much better than my Russian, so don't worry about this too much :P

Also, could you tone down the site theme? The scrolling sidebar/topbar etc is really buggy and keeps getting in the way of your text. I'd suggest just using jekyll or something with Github pages or maybe just run your own HTTP server to serve it up.

I use blogspot because of slash dot effects. theme is really buggy, shame on blogger. Changed to simple one.

Fair enough, that was my biggest gripe anyways.

They're not new attact vectors but I enjoy Egor's posts and call outs. If you're a web dev making these mistakes then you need to take the time to learn about basic web security. You can't count on frameworks to abstract it all away for you, you have to understand how it works.

The same CSRF rules apply to GET and POST requests. If you're using proper CSRF protection it doesn't really matter if you do a GET or POST. The only real difference is that GET requests will be visible to every piece of hardware between the user and your server whether you're using SSL or not and will most likely be logged. Also you can upload files via POST.

While it's easier to make users send harmful GET requests by putting the URL in an image you can simply use JS to POST a hidden form. Thinking POST is inherently more secure is a false assumption. Both are equally vulnerable to CSRF.

Just FYI, the entire HTTP request is encrypted with SSL, including the URL. Otherwise https://www.google.com/?q=sensitive+query would be pretty pointless!

Oops, you're right. The only data exposed would be the hostname and port. Any query data would be encrypted.

I am not 100% convinced. I would have thought GET would be slightly less secure than POST. You use the JS example. If I were running NoScript or something where JS didn't work someone could use <img src="follow?username=value" > and it would run. While if it used POST, it would be slightly more secure (for a user being exploited).

It's entirely possible I mis-understand the whole thing and please correct me if I am wrong (and explain why).

Are you sure that ssl exposes url? I was under the impression that the connection handshake happened before any of the rest of the http protocol.

you just rewrote my post in other words. Of course I know how to send POST and I call this a little bit "new" vector only because even good developers(check showcases to make sure) use 'match' and forget to specify :via. And it looks OK to them because they miss the point - all GET requests skip protect_from_forgery filter in Rails - this is how it works.

>If you're a web dev making these mistakes then you need to take the time to learn about basic web security.

Honestly, I didn't say it loudly but I meant it. Not understanding GET/POST makes person low skilled dev.

> all GET requests skip protect_from_forgery filter in Rails

Now that's interesting, I wasn't aware of that. It makes sense due to the Rails convention being GET is only for fetching data. So this does make GET more insecure than POST for Rails.

Egon, any chance you'll take a look at Django and give it the same level of scrutiny you are giving to rails?

Currently I'm feeling very superior about how my framework is more secure than yours, but I'd love to see you shoot down that silly notion.

I think the fact that Django's url mapper doesn't make it easy to dispatch based on the method means that Django wouldn't score to highly against this particular issue. It's easy to put

    if request.method not in ('POST', 'PUT', 'DELETE', 'PATCH'):
        ...

at the top of every method (or use a decorator), but it'd definitely be nice if this were part of the url dispatcher.

This is true, but at the same time Django doesn't make it easy to mix up get and post params. Calling request.POST['param'] will raise an exception on a GET request.

Finding rails issues and then saying "they kinda sorta apply to Django" isn't as interesting as finding real Django issues.

request.REQUEST? Admittedly, people don't use it that much, but it's definitely there.

Thanks for this information! Really, it is $_REQUEST as in PHP.

To yummyfajitas - your message is 50% trolling. Will you allow me to troll a little bit? I used django and scrapy few years ago and despite the fact it was better than PHP I would not even dare to compare it with Rails. Rails is that superior I don't even have words to explain it :D Conclusion: I'm not interested in Django and its bug because I love rails and wanna make it more secure anyways. Sorry, but Django is way less convinient to use. Security is another story though.

Sorry, I didn't intend to sound like I was trolling, my comment "I'm feeling very superior" was intended to be facetious.

Also, if you ever manage to put into words why you prefer Rails, I'd love to read it. Django and rails seem pretty similar to me, but I didn't put much effort into learning rails. But maybe I'm just experiencing the blub paradox.

So I don't think yummyfajitas was trolling, but he obviously likes Django. Like him, I like Django as well, but I happen to think that a healthy criticism of any popular framework is worthwhile. I think it's telling that AFAIK all more recent python frameworks bake in method dispatching to the url dispatcher.

It's not in the URL dispatcher in Django, but the class based views have separate methods that are called depending on the HTTP verb such as get(), post() etc.

It was a problem with the older function based views, and there was a lot of boilerplate to ensure the correct HTTP verb was being used.

When using class-based views, this is already built in. You can do something this:

    from django.views.generic.base import View
    from django.http import HttpResponse
    
    class MyView(View):
        def get(self, request):
            return HttpResponse("Get request")

        def post(self, request):
            return HttpResponse("Post Request")

The functionality to make this "just work" (dispatch based on method) is built into django.views.generic.base.View.dispatch.

Would love to see some more information on best practices for routing. I've always used http://guides.rubyonrails.org/routing.html as a go-to resource on routing, and it uses 'match' statements throughout. Is the suggestion here to just change all match statements to the intended REST verb?

You should be using resource-based routes if you've already got the RESTful controller thing going.

The match statement is good for the occasional odd cases, but it has an :via option that can be used to restrict which HTTP actions it is allowed to match.

Edit: corrected below...thanks!

Fix - not :as but :via. Using match :via => :post is worse than just post 'data', it is obvious btw :)

and there are almost no odd cases in fact. I would propose special route named "not_found" or "default" like route to be called if no routes found. Nice idea IMO, what do you think?

Thanks, it is early :)

I had to use match when moving from Rails 2 to Rails 3 and parts of the route did not match the controller at all. But I think it was mostly because the upgrade plugin recommended it.

The only thing I don't like about the Rails routing guide is how much it talks about having a default route of `match ':controller/:action/:id'`. I would still use `match`, but any actions that are meant to be posts should add `:method => :post` to them. And ideally use `resources` where ever it makes sense.

You can simply write post instead of match to require a POST request.

You can use most all of the verbs:

   get    '/foo'     => 'foos#index',   as: :foo
   post   '/foo'     => 'foos#new',     as: :new_foo
   get    '/foo/:id' => 'foos#show',    as: :show_foo
   put    '/foo/:id' => 'foos#edit',    as: :edit_foo
   delete '/foo/:id' => 'foos#destroy', as: :destroy_foo

or scope them if you'd like:

    scope controller: "foo", constraints: { id: /0-9]+/ } do
      get    '/foo'   => :index, as: :foo
      post   '/foo'   => :new,   as: :new_foo
      put    ...
      delete ...
    end

Fix: not :method but :via :)

And, again! PLEASE use method "data" instead of match :via => method. It's much better and looks clean.

In exactly what sense do you think that's better?

it does absolutely same job(you can read sources) but requires less symbols and looks RESTful - fair enough, isn't it?

Sorry, you're right about the :via, I must have misremembered.

Right, so what this post is saying is that for the most part the evil is in using this default route, leaving open controller actions that should only be accessible as posts. Will have to look more into the docs, but it's good to know that the methods specified in the guide have some caveats.

But all rails 2.3 apps use map.connect == are vulnerable(holy shi~!)

Rails 2.3 had "verify :method => :post" which was supposed to be used in controllers. This is nothing new.

Supposed to be, but rarely was. Throw a bunch of new programmers at a framework, and insecure-by-default becomes insecure.

Rails 2.3 also has:

    map.connect "/foo", :controller => "foos", :action => "index", :conditions => { :method => :get }

Which is the "correct" way to do verb-constrained non-resource routes. Not ever routing the wrong verb is even better than checking it in the controller.

call me a bad coder but when I used rails 2.3 I had never seen it in production code. I was too young though - may be you're right.

Anyways rails 2.3 had no csrf protection

call me a bad coder but when I used rails 2.3 I had never seen it in production code.

Ok, then you're a bad coder. All Rails applications I've ever worked on had those checks and the documentation for Rails made it pretty clear that verify :method should be used for non-GET requests.

Anyways rails 2.3 had no csrf protection

You really need to check your facts. Rails has had protect_from_forgery since at least 2.2 and it was enabled by default in ApplicationController. Rails < 2.3.10 did not do the verification for AJAX requests, but this was changed in 2.3.11.

Added in 2007: https://github.com/rails/rails/commit/4e3ed5bc44f6cd20c9e353... and made the default shortly after.

Appreciate your info! Really, I messed with facts - in 2011 it was made for AJAX :) You are right!

Ops FIX: you should call bad coder not me but people whom code I had been reading years ago.

And anyways burke is right: >Supposed to be, but rarely was. Throw a bunch of new programmers at a framework, and insecure-by-default becomes insecure.

btw verify method: :post is nice to have but obviously uglier than current routes.rb DSL.

Security aside, the catchall routing directive is a maintenance nightmare. The router is precisely where you should catch buggy routes, as well as document valid ones so they come up in rake routes.

blogger and tons of javascript loading wheels.. I just want to read som text, google how hard can it bee!!!?

blogger fail. changed to simple theme

Reply on news.ycombinator.com