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
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_accessiblestarting 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
superuserflag to your
Usermodel 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
Using a whitelist would mean you would have to enable this behaviour explicitly in the profile controller.