Protecting ActiveRecord Associations

I am working on a REST API and I’ve noticed mass assignment can really
be a huge security hole if you don’t take proactive measure. For
example:

class User < ActiveRecord::Base
has_many :categories
end
class Category < ActiveRecord::Base
belongs_to :user
end

If you call @category.update_attributes in your controller, you leave
the door open for someone to pass in :user_id or :user parameters and
change the ownership of the category.

Of course Rails has the conventions to prevent this from happening with:

attr_protected :user_id, :user

But you don’t see much written about protecting attributes. Some models
will allow you to use validations to avoid security holes, but I’m
finding most of my belongs_to’s need to be protected.

In order to clean up my code and avoid stupid security holes, I’ve
thrown together a plugin to explore this problem. You can check it out
at Google Code Archive - Long-term storage for Google Code Project Hosting. So far I’ve only added
a belongs_to_protected method which will call the regular belongs_to and
protect the related attributes.

If there’s another way people are addressing this problem, let me know.
Otherwise if anyone else wants to use this plugin, I’ll add support for
the rest of the ActiveRecord associations. I’m also thinking of making
an _immutable version which will allow you to only set an attribute
once. Is there another convention in Rails for doing this?

Here’s my initial blog post on the topic:
http://tuples.us/2007/05/12/activerecord-associations-and-the-unfortunate-matter-of-security/

Jordan
http://jordan.mckible.com

I saw your initial blog post on the subject and it got me wondering
if it wouldn’t be better to make associations private by default.

-faisal

Hi –

On Sun, 13 May 2007, Jordan McKible wrote:

finding most of my belongs_to’s need to be protected.
Have you tried attr_accessible? It’s sort of the mirror image of
attr_protected – you provide a whitelist instead of a blacklist.

David


Q. What is THE Ruby book for Rails developers?
A. RUBY FOR RAILS by David A. Black (Ruby for Rails)
(See what readers are saying! http://www.rubypal.com/r4rrevs.pdf)
Q. Where can I get Ruby/Rails on-site training, consulting, coaching?
A. Ruby Power and Light, LLC (http://www.rubypal.com)

finding most of my belongs_to’s need to be protected.
Have you tried attr_accessible? It’s sort of the mirror image of
attr_protected – you provide a whitelist instead of a blacklist.

Good idea. You’d just have to remember to maintain the whitelist if the
model ever changed. Probably not a big deal, but it would be one more
thing to remember. From an aesthetic prospective, I think I’d prefer
the protection rolled up into the association.

Are you familiar with any pattern that only allows nil attributes to be
updated? It seems like that’s a closer fit to what some association
protection is trying to accomplish.

Jordan
http://jordan.mckible.com

On May 14, 2007, at 12:28 AM, Jordan McKible wrote:

Good idea. You’d just have to remember to maintain the whitelist
if the
model ever changed. Probably not a big deal, but it would be one more
thing to remember.

One more thing for the automated tests to catch :slight_smile:

Are you familiar with any pattern that only allows nil attributes
to be
updated? It seems like that’s a closer fit to what some association
protection is trying to accomplish.

You might be able to do something with faux accessors.

-faisal

On 14/05/07, [email protected] [email protected] wrote:

I agree about the remembering and aesthetics; it definitely means
you’re manually maintaining a kind of parallel configuration on top of
the basic business logic. But keep in mind that it’s not just
associations that are vulnerable: someone could also maliciously
insert a change using, say, a class-variable accessor. (Unless this
was tightened up in edge recently? But I don’t think so.)

This was fixed with some release ago at least for class accessor
defined from within rails itself AFAIK

What I’ve done to make more strict the mass assignment methods is a
plugin (not published) that just compiles by default attr_accessible
with the actual db attributes of the object, stripping all the foreign
keys.
The plugin offers a facility to add/drop to/from attr_accessible
custom method that you want to make available in mass assignment.
This is quite easy to implement and gives a reasonably safe default to
start from.

Paolo

Hi –

On Mon, 14 May 2007, Jordan McKible wrote:

finding most of my belongs_to’s need to be protected.
Have you tried attr_accessible? It’s sort of the mirror image of
attr_protected – you provide a whitelist instead of a blacklist.

Good idea. You’d just have to remember to maintain the whitelist if the
model ever changed. Probably not a big deal, but it would be one more
thing to remember. From an aesthetic prospective, I think I’d prefer
the protection rolled up into the association.

I agree about the remembering and aesthetics; it definitely means
you’re manually maintaining a kind of parallel configuration on top of
the basic business logic. But keep in mind that it’s not just
associations that are vulnerable: someone could also maliciously
insert a change using, say, a class-variable accessor. (Unless this
was tightened up in edge recently? But I don’t think so.)

Are you familiar with any pattern that only allows nil attributes to be
updated? It seems like that’s a closer fit to what some association
protection is trying to accomplish.

I don’t think there’s any facility for that at the moment. Maybe one
could write an attr_gateway method, or something, with a block.

David


Q. What is THE Ruby book for Rails developers?
A. RUBY FOR RAILS by David A. Black (Ruby for Rails)
(See what readers are saying! http://www.rubypal.com/r4rrevs.pdf)
Q. Where can I get Ruby/Rails on-site training, consulting, coaching?
A. Ruby Power and Light, LLC (http://www.rubypal.com)

Hi –

On Mon, 14 May 2007, Paolo N. wrote:

This was fixed with some release ago at least for class accessor
defined from within rails itself AFAIK

Cool.

What I’ve done to make more strict the mass assignment methods is a
plugin (not published) that just compiles by default attr_accessible
with the actual db attributes of the object, stripping all the foreign
keys.
The plugin offers a facility to add/drop to/from attr_accessible
custom method that you want to make available in mass assignment.
This is quite easy to implement and gives a reasonably safe default to
start from.

That sounds like a great solution. Are you interested in publishing
the plugin?

David


Q. What is THE Ruby book for Rails developers?
A. RUBY FOR RAILS by David A. Black (Ruby for Rails)
(See what readers are saying! http://www.rubypal.com/r4rrevs.pdf)
Q. Where can I get Ruby/Rails on-site training, consulting, coaching?
A. Ruby Power and Light, LLC (http://www.rubypal.com)

Hi –

On Mon, 14 May 2007, Paolo N. wrote:

I agree about the remembering and aesthetics; it definitely means

the plugin?

If I can see there’s some interest around it, I might, if I have
permission from my employer and if I’ve time to arrange to actually
publish it on rubyforge.
But I have to warn it works well only if the app follows AR naming
convention of foreign keys.

You could grab the keys dynamically with reflections or
reflect_on_all_assocations. That might even be easier than
calculating them yourself from table names (if that’s what you’re
doing).

David

Paolo

David


Q. What is THE Ruby book for Rails developers?
A. RUBY FOR RAILS by David A. Black (Ruby for Rails)
(See what readers are saying! http://www.rubypal.com/r4rrevs.pdf)
Q. Where can I get Ruby/Rails on-site training, consulting, coaching?
A. Ruby Power and Light, LLC (http://www.rubypal.com)

Paolo N. wrote:

What I’ve done to make more strict the mass assignment methods is a
plugin (not published) that just compiles by default attr_accessible
with the actual db attributes of the object, stripping all the foreign
keys.

The converse would also be helpful: an update_attributes_unsafe method
that allowed unrestricted mass assignment for internal processing,
allowing several separate assignment lines to be compressed into one,
avoiding the puzzlement that can occur when use of update_attributes
for such a multiple assignment fails to work.


We develop, watch us RoR, in numbers too big to ignore.

On 14/05/07, [email protected] [email protected] wrote:

the basic business logic. But keep in mind that it’s not just
plugin (not published) that just compiles by default attr_accessible
with the actual db attributes of the object, stripping all the foreign
keys.
The plugin offers a facility to add/drop to/from attr_accessible
custom method that you want to make available in mass assignment.
This is quite easy to implement and gives a reasonably safe default to
start from.

That sounds like a great solution. Are you interested in publishing
the plugin?

If I can see there’s some interest around it, I might, if I have
permission from my employer and if I’ve time to arrange to actually
publish it on rubyforge.
But I have to warn it works well only if the app follows AR naming
convention of foreign keys.

Paolo

dear sender,
i´m out of the office until may 29th.
your email will not be forwarded.
for urgent stuff please contact [email protected]
kind regards,
alexander

Ok, next week I’ll be trying to publicly release another project of
mine, but after that I’ll seriously look into publishing the plugin so
we can look more in detail on how to improve it.

Paolo