CODEWITHSUNDEEP
ruby-on-railsruby-on-rails-3ruby-on-rails-3.2attr-accessible
pratski
pratski

Reputation: 1488

Issue and clarification needed with attr_accessible

There is so much written about the security threat of attr_accessible that I am beginning to wonder if I should even have any attributes in it. Here is the issue. I have a Message model which has the following:

attr_accessible :body,:sender_id,:recipient_id

I do not have the update or edit action in my messages_controller. With the new and create action I am able to create a new message and send it to a recipient. Only users who have logged in and meet certain conditions can message each other. I do that with the help of a before_filter and the conditions work fine. The message is stored and can be viewed by the sender and the recipient. Perfect!

The question I have is that since :body,:sender_id,:recipient_id are included in attr_accessible, can a malicious user somehow change the :body,:sender_id,:recipient_id of the original message? Should I just add these attributes to attr_readonly as well so they cannot be modified once saved?

This question has been haunting me for practically all my models.

Upvotes: 1

Views: 51

Answers (2)

Lazarus Lazaridis
Lazarus Lazaridis

Reputation: 6029

Well, it depends on how your are creating your model's instance. If you use:

FooModel.create(params[:foo])

then yes, your are not secure because a logged in user may pass additional parameters to the request even if you don't provide explicitly form fields for those attributes.

So, for your case, anyone posting to your "create" action with sender_id, recipient_id (values in the request) will be able to change them unless you take care about this assignments in your action.

Upvotes: 1

aromero
aromero

Reputation: 25761

can a malicious user somehow change the :body,:sender_id,:recipient_id of the original message?

This would depend on other things rather than attr_accesible. attr_accesible will only filter which fields are allowed to be updated using mass assignment. Since you say you don't have any update action, then no, there is now way a user can edit a message since you always create a new Message through you create action.

But there is something you need to care about. What is sender_id? If you do have users in your app and they send messages to each others, then sender_id should not be an accessible field, since this will allow users to send messages on behalf of other users. You probably want to keep that field off the attr_accessible list and do something like this:

m = Message.new params[:message] # body and recipient_id
m.sender_id = current_user.id # this is not mass assignment
m.save
.....

Upvotes: 3

Related Questions