You Should Be Whitelisting Parameters

In the rails world, the following is a common pattern:

# Blacklisted Request Parameters

class BlacklistingUsersController < ApplicationController
  def update
    params[:user].delete(:admin)
    user = User.find(params[:id])
    user.update_attributes(params[:user])
  end
end

The intention here is to delete the :admin parameter so that a user isn't able to switch this parameter to 'true' on their own account.

This is an example of blacklisting, or rejecting known bad. In general it's considered more effective to whitelist parameters i.e. accepting known good.

In all situations where you are intending to filter parameters, it is a great deal safer in the long run to whitelist parameters instead of blacklist. An example of how you might do this in a Rails controller might be:

# Whitelisted Request Parameters, as suggested by DHH

class WhitelistingUsersController < ApplicationController
  def update
    user = User.find(params[:id])
    user.update_attributes user_params
  end

  private

  def user_params
    params[:user].slice :first_name, :last_name, :email
  end
end

Here we've specified exactly which parameters we intend to pass to update_attributes rather than used whatevers left after removing the :admin parameter.

Isn't attr_accessible enough?

No. The fundamental problem with relying on attr_accessible is that filtering request parameters is a controller concern. That's not to say there might be a neater way of expressing a parameter whitelist, but it really needs to be on a per-controller or even per-action basis, rather than set once globally for wherever the model is used.

Having a single, globally set list of whitelisted parameters for a given user model begins to make a lot less sense when you're manipulating your model from multiple parts of your system. Taking are users controller example further:

  • Some users may have admin rights over other users. So the parameter whitelist for 'admin' users updating regular users is going to be different to regular users updating themselves (An admin might have the ability to 'promote' a regular user to an admin).

  • Rake tasks and scripts tend to have free access to all parameters. A common theme in rails codebases is attr_accessible starting out quite conservative, but then growing steadily as functionality for higher level users and utility scripts is built out.

In both of the above cases, having a parameter whitelist at the controller level would go a long way towards preventing inadvertent security vulnerabilities.

More benefits of whitelisting over blacklisting

This has the following benefits over blacklisting:

  • The code is explicit about the parameters it's sending to the model. A bit of a tautology, but at the very least using a whitelist makes it clear to anyone reading the code later exactly what parameters you're intending to send to a given model being updated.
  • Updating new, sensitive parameters in your model won't implicitly be allowed by a whitelist. For example if you add a superuser flag to your User model and forget to add it to your parameter blacklist on your update profile action, you'll implicitly (and perhaps unknowingly) be giving your user the ability to flip their own superuser flag.

    Using a whitelist would mean you would have to enable this behaviour explicitly in the profile controller.