Why slicing the params hash poses a security issue

Question

The official way of preventing security risks with mass-assignment is using attr_accessible. However, some programmers feel this is not a job for the model (or at least not only for the model). The simplest way of doing it in a controller is slicing the params hash:

@user = User.update_attributes(params[:user].slice(:name))

However the documentation states:

Note that using Hash#except or Hash#slice in place of attr_accessible to sanitize attributes won’t provide sufficient protection.

Why is that? Why a whitelist-slicing of params does not provide enough protection?

UPDATE: Rails 4.0 will ship strong-parameters, a refined slicing of parameters, so I guess the whole slicing thing was not so bad after all.

Answer 1

The problem with slice and except in controller might occur in combination with accept_nested_attributes_for in your model. If you use nested attributes, you would need to slice parameters on all places, where you update them in controller, which isn't always the easiest task, especially with deeply nested scenarios. With using attr_accesible you don't have this problem.

Answer 2

As of Rails 4, slicing the parameters will be the preferred method of dealing with mass assignment security. The Rails core team has already developed a plugin to deal with this now, and they are working on integrating support for nested attributes and signed forms. Definitely something to check out: http://weblog.rubyonrails.org/2012/3/21/strong-parameters/

Answer 3

Just removing the :name from the params hash works to prevent setting that attribute for that action. It works only for the actions you remember protecting.

However, this practice doesn't protect you from abuse using all the methods automatically added for associations.

class User < ActiveRecord::Base
  has_many :comments
end

will leave you vulnerable for someone setting the comments_ids attribute, even when you delete the comments attribute from params.

Since there are quite a lot of methods added for associations, and since they might change in the future, the best practice is to protect your attributes on the model using attr_accessible. This will stop these kind of attacks most effectively.

Answer 4

Interesting gist from DHH on slicing in controller vs whitelisting alone:

https://gist.github.com/1975644

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

Comment reinforcing the need to manage this within the controller:

https://gist.github.com/1975644#gistcomment-88369

I personally apply both - attr_accessible with slice to ensure nothing unexpected gets through. Never rely on blacklisting alone!

Answer 5

@tokland your last comment is not correct to some extend. Unless your website has the browser as the only entry point where data comes in and goes out.

If your webapp has an API or communicates with other API's protection on the controller level leaves holes behind it and all data from other sources is not sanitised or checked. I recommend keeping the things as they are, turning on mass-assignment protection in application.rb and advancing ActiveSupport FormHelpers to work like Django/Python style.