attr_accessible and Security in Rails

This is a story of how easy it can be to inadvertently put security flaws in your programs.

Given that I had implemented authentication and authorization, I wanted a place where users could edit their profile information. However, I wanted to always keep the user's original name and email address that was provided by OpenID authentication. So my thought was that I would provide the user with a "preferred email address" and a "display name" which I would pre-populate based on what OpenID provided, but which they could change to anything they wanted. I'd still keep the original information, but just not ever show it to them.

Step One - New Database Columns
This was just a matter of creating a new migration:

class AddDisplayNameToUser < ActiveRecord::Migration
  def self.up
    add_column :users, :display_name, :string
    add_column :users, :preferred_email, :string
  end

  def self.down
    remove_column :users, :display_name
    remove_column :users, :preferred_email
  end
end

and running

rake db:migrate

Step 2 - Create Default Values in Model

Pre-populating these new columns was easy, all I needed was to use the "after_initialize" hook in ActiveRecord. To do this, I added the following lines to my User model:

after_initialize :init

def init
  self.display_name ||= "#{self.last_name}, #{self.first_name}"
  self.preferred_email ||= self.email
end

Step 3 - Create Web Pages

I created a Profile resource with the following line in routes.rb

resource :profile, :only=>[:show, :edit, :update]

In the "show" action, I show the user their display name and preferred email. The edit page allows them to edit these two pieces of information. The HAML for this is:

%h1 Edit Your Profile
= form_for @user, :url=>profile_path do |f|
  = f.label :display_name
  = f.text_field :display_name, :size=>50
  %br
  = f.label :preferred_email, 'Preferred email address'
  = f.text_field :preferred_email, :size=>50
  %br
  = f.submit 'Update Profile'

The update method in the controller, which gets called when the form is submitted looks like:

def update
  current_user.update_attributes params[:user]
  redirect_to profile_path
end

And with that, I have a page where a user can update their own information. I'm done, right?

Security Flaw

Do you see the security flaw?

The problem is one that frequently comes up in client/server settings, particularly on the web. The server is putting too much trust in the client. It's all well and good to restrict the user's edit page to just the Display Name and Preferred Email fields, but a wily user will just bypass the web browser and directly make an HTTP request with parameters set to change fields that we don't want changed, like the :identifier_url.

So how do we fix this? Well the obvious way is to replace the line

current_user.update_attributes params[:user]

in the update method with code that explicitly just sets the fields that we care about. But then we lose the convenience of letting this Rails library method do the work for us. Luckily there's a solution, and I am sure the title of this post has not given you any hint to that solution. If you add the line:

attr_accessible :display_name, :preferred_email

to your User model class, you are telling Rails to allow those fields to be updatable via a mass assign command like update_attributes. More importantly, as soon as you've declared at least one field as attr_accessible, then all of the non-declared fields can't be update this way.

Take Home Message

Never Trust the ClientI am sure you know this already, but it bears repeating: Never Trust the Client. There is nothing stopping a malicious user from writing their own client and sending your server any data they want. Unfortunately, this requires eternal vigilance and learning. Sometimes holes, like the one above, exist not because of code you wrote, but because of the code you didn't write. If I hadn't read about this exact situation in Michael Hartl's Ruby on Rails 3 Tutorial, I might never have even realized the security hole I had created.