Is it safe to use find params[:id] in Rails?

Question

Expressions like the following are common in Rails:

@project = Project.find params[:id] # example 1

@project = current_user.projects.find params[:project_id] # example 2

However I realize that find also accepts arrays! That would mean unexpected and potentially dangerous behavior of code, which is written with the assumption that @project is a single project, not an array.

Now the question is:

• should I typecast params[:id].to_s every time I use it in find?
• should I use strong parameters with find in order to avoid arrays? find params.permit(:id)[:id])? (better than to_s if you don't want to convert nil to "")
• are there any other alternatives or common practices?

---

The above vulnerability seems to be present only if the routes doesn't define a param with that name.

For example:

# SAFE routes.rb
resources :projects

# projects_controller.rb
Project.find params[:id]

The query /projects/3?id[]=4&id[]=5 produces only {id: 3} as a param. This would make using Project.find params[:id] safe: however I cannot find any documentation for this behavior and I don't know if it's safe to rely on it: maybe it's just by chance.

Furthermore the following is not equivalent at all and would create a vulnerability in the controllers:

# Likely UNSAFE routes.rb
# E.g.: 
# /projects?id=3 => params = {id: 3}
# /projects?id[]=3&id[]=4 => params = {id: [3, 4]}
put '/projects' => 'projects#update'

Schwann

personally, I use find_by(id: params[:id]) more often. find would break the code if it can't find with given value and results ActiveRecord::RecordNotFound error. whereas find_by will return nil if it can't find anything, and I believe that it's safer in some senses – Past

The nature of the vulnerability you describe appears to be "an attacker can generate a 500 server error". Is there a particular sort of escalation you are worried about? – Ardeen

@JimVanFleet For example Token.find(params[:token]) allows an attacker to try thousands of tokens with just one call – Schwann

@Past It's idiomatic Rails to use find and let the exception generate a 404 error. – Hadron

Answer 1

It depends. How are your params coming in? Is your params[:id] and params[:project_id] arriving as route params? If so then you're safe.

Otherwise you want to use strong params - which should cover you.

Finally, you probably want to use #2 because you don't want users gaining access to projects they don't belong to.

Marginalia