A couple days ago I was contacted by Adam Doupé regarding a vulnerability he had been researching for his PhD. He had been scanning Github projects for the vulnerability and had found what could possibly be an issue on Fluttrly.

While Fluttrly was actually unaffected, the vulnerability is a serious one. When using a web framework such as Rails or Django, your “redirect” statement actually just generates a Redirect Response. This does not halt the execution chain which leaves the rest of your code to continue execution when you probably wanted it to stop.

Here’s an example from Adam’s blog:

class TopicsController < ApplicationController
  def update
    @topic = Topic.find(params[:id])
    unless current_user.is_admin?
      redirect_to "/"
    end
    if @topic.update_attributes(params[:topic])
      flash[:notice] = "Topic updated!"
    end
  end
end

With this code, a user can still change the topic even if they aren’t an admin because authorization of the request isn’t handled properly. The code looks correct upon first glance, but it is subject to anyone changing the topic.

The solution Adam suggets for this is to redirect and return on the same line in your controller. That’s fine if you need to do a redirect in your controller action, but it’s not a best practice, at least not for Rails.

The best remedy for this is simply to use filters in Rails. With a before_filter, you can check if the user is an admin and redirect accordingly:

before_filter :admin_required

def admin_required
  redirect_to "/" unless current_user.is_admin?
end

This will halt the chain before the code in update begins execution because of how Rails’ before filter’s work.

While this is just an example before_filter, you should really check out using a Rails authorization gem like CanCan. These gems allow you to configure user levels and the functionality they are allowed to execute. They provide a much better way to manage and configure authorization without overhead of having all of the code to handle this in your application.

I’m glad Adam is disclosing this to people. It’s something that is easily overlooked and it’s good to raise awareness of these things to new Rails developers. :D

comments powered by Disqus