Michael de Silva's Blog

Software Engineer. Rubyist and Roboticist.

Michael de Silva's Blog

Software Engineer. Rubyist and Roboticist.

Github "Hacked" by Homakov: The Aftermath...

There's a rather lengthy Gist on Github detailing what went into Homakov's hack; it's fair enough to call this a hack as it did allow him to assign his public SSH-key to a member of the Rails core team and cause a bit of a stir.

It's worthy to note that Model.new and Model.create are equally susceptible as obj.update_attributes when a hash of arguments are passed to them.

While I've been ailing away in the ER for a past few days — yes, I missed out on all the fun — I was wondering if doing something similar to a .reject on the params hash was worth it... but ah, reject is an array method and the equivalent method in the Hash domain is slice!

DHH has proposed the use of slice as per the following example

class PostsController < ActionController::Base
  def create
    Post.create(post_params)
  end

  def update
    Post.find(params[:id]).update_attributes!(post_params)
  end

  private
    def post_params
      params[:post].slice(:title, :content)
    end
end

...and he points out this can easily be made Controller state aware as well

def post_params
  if current_user.admin?
    params[:post].slice(:title, :content, :published)
  else
    params[:post].slice(:title, :content)
  end
end

One should not miss reading this Proposal for Improving Mass Assignment by Yehuda Katz which does confirm DHH's approach (in fact DHH agrees with Yehuda's suggestion of this being handled in the controller).

Here's quoting Katz's conclusion,

The core problem with Rails mass assignment is that attribute protection is an authorization concern. By moving it to the controller, we can have smart defaults (like signed fields in form_for) and in more advanced cases, make it easier to decide what fields are allowed on a per-user basis.

By moving it into the correct place, we will probably find other nice abstractions that we can use over time to make nice defaults for users without compromising security.

I also suggest watching the revised railscast from 2007 titled "Hackers Love Mass Assignment" which elucidates the attack quite clearly.

One approach is to add the following initializer, config/initializers/disable_mass_assignment.rb to your projects, paying particular attention to the associated warnings,

ActiveRecord::Base.send(:attr_accessible, nil)

...or simply enable the application config option in newer Rails apps config.active_record.whitelist_attributes = true. This setting will be default for all newly generated apps as seen via a commit in the 3-2-stable branch.

comments powered by Disqus