Is your Rails application safe?

Posted by Eric Chapweske
on Monday, September 22

Rails provides many great security features. It’s design can also create significant security holes. In the case of ActiveRecord’s mass assignment vulnerability, the security issues are more servere and widespread than many of us recognize.

Nearly every open source Rails application I’ve seen is vulnerable, and most closed source ones as well. There’s some great solutions for protecting your application from attack, but first, the problem:

The Problem

By default ActiveRecord allows visitors access to any writer method, that is, any method ending with an equal sign. This comes courtesy of the ActiveRecord::Base#attributes= method, which is used internally by the main methods that handle creating and updating records, including new(), create(), and update_attributes().

The way most applications are designed means that whatever data a visitor sends to the server will likely find its way through the attributes=() method, and if not protected, ActiveRecord will happily update the records based on what was sent. In less technical terms: ActiveRecord is insecure by default.

As an example, let’s look at a request against vulnerable code:

1
2
3

# The request
$ curl -X PUT -d "order[price_in_cents]=0" example.com/orders/225
app/models/order.rb
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

class Order < ActiveRecord::Base
  # Table name: orders
  #  id          :integer(11)     not null, primary key 
  #  price_in_cents     :integer(11)
  #  user_id     :integer(11)     
  #  state       :string(255)               

  has_many :line_items
  
  acts_as_state_machine :initial => :pending

  state :pending
  state :paid
    
  def name
    ... 
  end
  
  def shipped_on=(shipping_date)
    ...
  end

end
app/controllers/orders_controller.rb
1
2
3
4
5
6
7
8
9

class OrdersController < ApplicationController
  ...

  def update
    @order.update_attributes(params[:order])
  end

end
Pop quiz: which Order instance methods are exposed to the world?
  • Attributes generated from its table: price_in_cents=, user_id=, state=
  • Attributes generated by association macros: line_item_ids=
  • Other defined writer methods: shipped_on=

Ruby’s dynamic nature and ActiveRecord’s changing API make this excercise more of a guess than anything else. Does Rails 2.1 dynamicly generate different writer methods? Will Rails 2.2? How about the plugins and libraries the application relies on?

Theoritically, this isn’t a problem since ActiveRecord provides a solution out of the box: “Sensitive attributes can be protected from this form of mass-assignment by using the attr_protected macro. Or you can alternatively specify which attributes can be accessed with the attr_accessible macro”

The Reality

Naturally, profesional developers experienced with the framework use attr_accessible/attr_protected and don’t suffer from these problems. As a quick poll, here’s a few of the more popular open source code bases:

  1. Insoshi is second only to Rails as a top forked project on github. It’s a social networking app developed by a seed funded startup whose team includes the author of a very well-reviewed book on developing social applications in Rails.
  2. Mephisto, the Rails-based blogging application, which Railspikes runs on.
  3. Anonymous App is a large Rails project with seasoned developers. I’m withholding its details since it has security issues that are still being addressed.
  4. Rubyflow, the codebase of Peter Cooper’s very useful Ruby news aggregation site.
  5. Spree is a rapidly-maturing ecommerce project and powers RailsEnvy’s new screencast store.

Good projects. Professional developers. Every project except Mephisto is vulnerable. Any forum thread in Insoshi will raise exceptions and be unusable after the user_id is changed to a non-existent user. A similiar approach worked on Anonymous and Rubyflow. Since these projects lacked any strategy for handling this kind of problem, it’s highly probable that much more damaging attacks exist. One example: Spree’s public exposure of the 'state' attribute allowed me to make my order appear as though it was paid for when I hadn’t even entered my payment information. While these projects vary in terms of risk, in each case the cost of solving this issue is cheap when compared to the cost of cleaning up after an attack.

I’m singling out these applications because they’re popular and open source, but every project I’ve developed has experienced the same security issues. The only thing that seems to change is how much data is vulnerable and how important it is. It’s a difficult problem to manage. Retrofitting security on existing code is a very unpleasant experience. It’s easy to forget when developing new applications. Educating other developers on the problem has proved unreliable.

As an aside, I’m impressed by the response of the developers on these projects. Insoshi, Rubyflow, and Spree addressed the issue almost instantly after being informed. It was a reminder to me of how lucky I am to be involved in such a passionate, professional community. Michael Hartl of Insoshi went so far as to write a mass assignment auditing plugin and offers some great advice on how he ended up tackling the problem.

A solution

  1. Don’t use attr_protected. I haven’t seen a compelling use case for it. It’s functionality is confusing. It should probably be removed from ActiveRecord.
  2. Do use attr_accessible. Its white list approach forces an explicit decision on the mass assignablity of attributes. A rule of thumb: if an attribute shouldn’t be in a user submitable form, it shouldn’t be accessible.
  3. Review and audit. Even with attr_accessible, a developer can still shoot themselves in the foot without code audits and reviews. Even if the application is secure today, holes will eventually be introduced into the code. In addition to peer review, automated auditing tools are a great, inexpensive way to find such security problems.
  4. Make it automatic. Disable mass assignment by default, requiring attr_accessible to be specified for each attribute. I’ve taken this approach on maybe 5 projects now. Here’s how to do it:
config/initializers/disable_mass_assignment.rb
1
2

ActiveRecord::Base.send(:attr_accessible, nil)

It’s worked quite well, with the exception of two cases where I had to retrofit it on larger applications. That was a nightmare. I’ve been tinkering with a plugin that aims to reduce some of the problems caused by attr_accessible, and make retrofitting a more pleasant experience. It’s not production ready, but I think there’s some small improvements in it worth stealing.

The downside: it’s pretty much a guarantee that you’ll run into confusing bugs during development. This is a major problem for developers new to the framework, and is annoying for the more experienced. ActiveRecord used to raise exceptions in development when mass assignment was attempted with an inaccessible attribute. This was great, but there were a few complaints, and conflicts with ActiveResource, so the change was pulled.

A better solution?

An alternative approach worth exploring is the route taken by Merb, which decided this is the controller’s problem, and has a plugin providing params_accessible functionality. There’s a similar plugin for Rails . This approach may be especially appreciated by developers who want to add some level of protection to an existing application, since less code needs to change.

I’ve hesitated to use this on applications that use ActiveRecord, which has a bad habit of making methods part of the public api when they should be privately scoped (those ending in _id, _ids, _count, most enumerables, etc) Because of this, attr_accessible serves double duty by discouraging public use of writer methods that should be private. Not really the best excuse, and I’d like to give the params_protected approach a try on my next Rails project.

Regardless of the solution, the cost of designing applications to handle potential mass assignment abuse from the beginning is so much cheaper than attempting to retroactively address the issue. Rails should step up and encourage such design decisions. Whether it’s something as extreme as disabling mass assignment from the start, or an unobtrusive change like adding a commented out attr_accessible line in generated models, the risk shouldn’t be ignored.

Security Tools

There’s a few other related tools that look promising for developing securer code:
  • Tarantula: A fuzzing plugin that spiders your application looking for problems. Via Stuart Halloway’s post on Revelance’s blog: “It crawls your rails app, fuzzing inputs and analyzing what comes back. We have pointed Tarantula at about 20 Rails applications, both commercial and open source, and have never failed to uncover flaws.” Aaron Bedrak’s Rails Security Audit PDF on Peepcode devotes significant space to getting this up and running. It also covers a few of the common mistakes developers can make when using a framework like Rails, and that alone may make it a worthwhile read.
  • ratproxy: Happened upon this on Google’s excellent security blog . From their announcement post: “[ratproxy] is designed to transparently analyze legitimate, browser-driven interactions with a tested web property and automatically pinpoint, annotate, and prioritize potential flaws or areas of concern.”
  • Audit Mass Assignment: Scans ActiveRecord models looking for potential mass assignment mistakes.
  • Find Mass Assignment: Searches controller actions for likely mass assignment, and then find the corresponding models that don’t have attr_accessible defined.
References
Comments

Leave a response

  1. rickSeptember 22, 2008 @ 01:36 PM

    Mephisto is old enough that I made the same mistake and learned about it long before those projects started :) Applying attr_accessible (and attr_readonly) is now a common part of model development for me. Sometimes I even go as far as adding helper methods.

    
    class ForumTopic < AR::Base
      attr_accessible :title, :body
      belongs_to :user
      belongs_to :forum
    end
    
    class User < AR::Base
      def post_topic(options = {})
        forum = options.delete(:forum)
        options.delete(:sticky) unless moderates?(forum)
        topic = forum.topics.build(options)
        topic.user = self
        topic.save
        topic
      end
    end
    

    It’s handy for dealing with user-specific param logic like “only admins can set the sticky bit on a forum”. I’m not sure if this is a good approach, and I can’t ever decide where to put those model creation helper methods.

  2. Henrik NSeptember 22, 2008 @ 01:44 PM

    The raising in dev is here if you don’t have the ActiveResource issues: http://henrik.nyh.se/2007/10/whiny-protected-attributes

  3. Jason ScragzSeptember 22, 2008 @ 02:01 PM

    Anyone come up with an elegant way to deal with having a lot of attributes that should be protected from the public, but a subset of those need to be editable by admins? This is what I came up with (which needs to be changed to use attr_accessible).

    @@admin_editable_attributes = [:admin_editable_attr, :another_admin_editable_attr]
    attr_protected :really_protected_attr, *admin_editable_attributes
    def update_attributes_with_admin_editable(attrs)
      Model.admin_editable_attributes.each {|a| self.send(:"#{a}=", attrs[a]) }
      self.update_attributes(attrs)
    end

    It feels kinda dumb though.

  4. Michael HartlSeptember 22, 2008 @ 02:36 PM

    Thanks for giving me the heads-up about the mass assignment issues in Insoshi, and great work on this article!

  5. Luke FranclSeptember 22, 2008 @ 02:55 PM

    Jason—

    I just assign them manually where necessary, but something like what Michael came up with that bypasses attr_protected/attr_accessible would work.

    You can also do this directly by using the attributes= method, because it has a second parameter.

    obj.attributes=(params[:object], false) will bypass protection.

  6. KenSeptember 22, 2008 @ 02:57 PM

    Wouldn’t it be sufficient to never use update_attributes, create, or new with the params hash (or any non-literal)?

    It looks like the ‘update’ method which script/generate scaffold makes is vulnerable, which is unfortunate, but I just blow those away anyway.

  7. MinaSeptember 22, 2008 @ 05:49 PM

    Christopher Bottaro wrote an implementation of doing this stuff in the controller, as you mentioned Merb does. It offers both white lists, black lists, and deep structure filtering.

    Initial announcements here and here, code here.

  8. John GuySeptember 22, 2008 @ 06:19 PM

    What about attributes that should be set only once?

    For example… A username might be set on creation, but after that it should never be touched?

  9. ShaneSeptember 22, 2008 @ 07:59 PM

    Why not be explicit and use active supports hash slice on incoming params before mass assignment?

  10. MinaSeptember 23, 2008 @ 06:31 AM

    @Shane

    For the most part, that’s what the plugin I mentioned above (param_protected & param_accessible) does.

    In addition, it allows you to specify how a submitted complex structure (using the [ and ] notation) should be filtered.

  11. Michael DeeringSeptember 24, 2008 @ 05:32 PM

    @jason, I have used a very similar approach. I would suggest the following change to your from experiance.

    
    def update_attributes_with_admin_editable(attrs)
      attrs.stringify_keys!
      Model.admin_editable_attributes.each {|a| self.send(:"#{a}=", attrs[a]) if attrs.has_key?(a) }
      self.update_attributes(attrs)
    end
    
    

    You don’t want to be setting nils on missing keys…

  12. Craig BuchekOctober 02, 2008 @ 03:40 PM

    I’m starting to think more and more that DataMapper is the way to go, defining the attributes (schema) in the model instead of in the database. Especially if we’re going to have to list most of them in the model anyway. I like to use the attribute_models plugin, so I can see all the attributes when I’m in the model file, without having to switch over to the schema file. And attr_accessible is another reason to list most of the attributes in the model file. I’m also considering adding some meta-data for each attribute, like human column names, and other items that could be used to automatically generate forms for models.

    Then I think that if I’m using DM instead of AR, and HAML instead of ERB, why am I using Rails at all? (And I don’t care much for the way Rails does routing.) Why not just use Merb? For now, the answer is that Rails has a larger following, so it’s easier to find documentation and other developers.

  13. Trevor TurkOctober 05, 2008 @ 05:32 PM

    @John Guy

    You can use attr_readonly to make attributes accessible only on creation:

    http://api.rubyonrails.com/classes/ActiveRecord/Base.html#M001317

    I got a sneaky change into Active Record that allows you to raise an exception when attempts to mass assign attributes is made. You can set this to only work in the test environment. It’s Edge Rails for now, but here’s a snippet to enable it:

    http://gist.github.com/14948

    More detail:

    http://rails.lighthouseapp.com/projects/8994/tickets/804-refactor-protected-attribute-assignment-warnings

  14. Pete YandellOctober 05, 2008 @ 10:23 PM

    I wrote a blog post about using ActiveSupport’s slice method to explicitly choose the attributes for mass assignment in the controller.