How best to sanitize fields in ruby on rails

Question

I currently have a controller capturing some html from TinyMCE on the front end. If I tinker with firebug it is possible to submit script tags and inject alert messages etc on to the screen.

edit: Currently I am fixing this in the model by using the sanitize helper:

require 'action_view'

class NotesController < AuthApplicationController

  include ActionView::Helpers::SanitizeHelper
...
  def update
    params[:note][:content] = sanitize(params[:note][:content],
        :tags => %w(a object p param h1 h2 h3 h4 h5 h6 br hr ul li img),
        :attributes => %w(href name src type value width height data) );

    @note.update_attributes(params[:note])

This feels messy in the controller. Is there a better way? I.e. somehow integrate this ActiveRecord so I can easily specify to do this to this and other fields before saving in a similar way to validation?

Thanks for any suggestions.

edit:

Making some progress here.

Under my /Libs I have

module SanitizeUtilities
  def sanitize_tiny_mce(field)
    ActionController::Base.helpers.sanitize(field,
      :tags => %w(a b i strong em p param h1 h2 h3 h4 h5 h6 br hr ul li img),
      :attributes => %w(href name src type value width height data) );
  end
end

Then in my Models the code collapses to

class MyModel < ActiveRecord::Base
  include ::SanitizeUtilities
...
  before_save :sanitize_content
...
  def sanitize_content
    self.content = sanitize_tiny_mce(self.content)
  end

end

This seems to strip out unwanted markup without too much fuss.

Pretty new to rails so nervous I might be doing something wrong. Can anybody see potential drawbacks here?

Thanks again

The more usual way to handle this in rails is to accept any old crap from the client and save it (safely, careful to avoid SQL injection). Then later when it comes time to display, sanitise there. — noodl, Jan 19 '12 at 09:40
That seems odd to me, why would I want to commit dirty data? That would increase the work and chances of missing a sanitize if I read the data back in multiple places. — Chris, Jan 19 '12 at 10:32
@noodl Agreed with Chris on this one. Stripping out the data up front means you only have to incur this process once. Not stripping it out means you have to incur this process every time you display the data. And like Chris said, you might forget to protect a view. — iwasrobbed, Jan 19 '12 at 16:51

score 14 · Accepted Answer · answered Jan 19 '12 at 16:48

I think the way you are doing it is fine, but if you are using before_save then you could potentially still fail validations (since before_save is called after validations). Also, you don't necessarily have to put it into it's own module, it could just be a private method on your class.

Something like:

class MyModel < ActiveRecord::Base

  before_validation :sanitize_content, :on => :create

  private
    def sanitize_content
      self.content = sanitize_tiny_mce(self.content)
    end
    def sanitize_tiny_mce(field)
      ActionController::Base.helpers.sanitize(field,
        :tags => %w(a b i strong em p param h1 h2 h3 h4 h5 h6 br hr ul li img),
        :attributes => %w(href name src type value width height data) );
    end

end

presumably `on: [:create, :update]` too... any other best practices? — Meekohi, Mar 01 '17 at 20:44

score 3 · Answer 2 · answered Jun 03 '15 at 02:30

This question seems to be answered but for anyone coming to this you might want to consider using custom mutators to make this more transparent. Something like:

class MyModel < ActiveRecord::Base
  def content= content
    write_attribute(:content, sanitize_tiny_mce(content)
  end

  private

  def sanitize_tiny_mce content
    ActionController::Base.helpers.sanitize(field,
        :tags => %w(a b i strong em p param h1 h2 h3 h4 h5 h6 br hr ul li img),
        :attributes => %w(href name src type value width height data)
    );
  end
end

This will ensure the content is sanitized any time it's changed.

Overriding the setter is a bit cleaner than sanitisation in a callback. I wish Rails provided a macro for this, allowing to just pass a lambda or a method symbol. Maybe the Attribute API has that. — Epigene, Apr 28 '23 at 12:26

How best to sanitize fields in ruby on rails

2 Answers2

Linked