Search code examples
ruby-on-railsrubyruby-on-rails-3activemodel

ActiveModel: dangerous use of send()?


In RailsCast 219, the following code is offered for creating a class for ferrying data back and forth from forms, but without any ActiveRecord persistence:

class Message
  include ActiveModel::Validations

  attr_accessor :name, :email, :content

  validates_presence_of :name
  validates_format_of :email, :with => /^[-a-z0-9_+\.]+\@([-a-z0-9]+\.)+[a-z0-9]{2,4}$/i
  validates_length_of :content, :maximum => 500

  def initialize(attributes = {})
    attributes.each do |name, value|
      send("#{name}=", value)
    end
  end
end

I'm new to Ruby, but the send("#{name}=", value) seems like an invitation for an attacker to assign arbitrary values to arbitrary fields. Is this an issue? A few commenters asked similar questions, but no response.


Solution

  • When I asked a question related to the same RailsCast recently and was told that the initialiser was dangerous, but sadly no justification was given.

    Having delved a little deeper I now believe that the method does not introduce any security weakness for the reasons that jdoe eluded to in his comment to your question. The send method does not circumvent accessor methods, therefore the security of attributes is controlled by accessor declarations as normal.

    However, I would recommend a validation check to improve robustness against an attempt to assign inaccessible or non-existant attributes. Similar to Sergio's suggestion but more general:

    attributes.each do |name, value|
      send("#{name}=", value) if respond_to?("#{name}=")
    end