Merge pull request #76 from AyuntamientoMadrid/sanitize-debate-desc-71

Sanitizes debate desc

app/models/debate.rb

@@ -12,6 +12,8 @@ class Debate < ActiveRecord::Base
 
   validates :terms_of_service, acceptance: { allow_nil: false }, on: :create
 
+  before_validation :sanitize_description
+
   def likes
     get_likes.size
   end
@@ -32,4 +34,10 @@ class Debate < ActiveRecord::Base
     editable? && author == user
   end
 
-end
+  protected
+
+  def sanitize_description
+    self.description = WYSIWYGSanitizer.new.sanitize(description)
+  end
+
+end

app/views/debates/show.html.erb

@@ -13,7 +13,9 @@
 
                 <%= @debate.author.name %> <span>&nbsp;&bullet;&nbsp;</span> <%= l @debate.created_at.to_date %> <span>&nbsp;&bullet;&nbsp;</span> <%= pluralize(@debate.comment_threads.count, t("debates.show.comment"), t("debates.show.comments")) %>
             </div>
-            <p><%= @debate.description %></p>
+            <div class="debate-description">
+              <%= @debate.description.html_safe %>
+            </div>
             <p><%= render 'shared/tags', debate: @debate %></p>
         </div>
 

config/application.rb

@@ -24,5 +24,8 @@ module Participacion
 
     # Do not swallow errors in after_commit/after_rollback callbacks.
     config.active_record.raise_in_transactional_callbacks = true
+
+    # Add lib to the autoload path
+    config.autoload_paths << Rails.root.join('lib')
   end
 end

lib/wysiwyg_sanitizer.rb

@@ -0,0 +1,10 @@
+class WYSIWYGSanitizer
+
+  ALLOWED_TAGS = %w(p ul ol li strong em u s)
+  ALLOWED_ATTRIBUTES = []
+
+  def sanitize(html)
+    ActionController::Base.helpers.sanitize(html, tags: ALLOWED_TAGS, attributes: ALLOWED_ATTRIBUTES)
+  end
+
+end

spec/features/debates_spec.rb

@@ -45,6 +45,23 @@ feature 'Debates' do
     expect(page).to have_content I18n.l(Date.today)
   end
 
+  scenario 'JS injection is sanitized' do
+    author = create(:user)
+    login_as(author)
+
+    visit new_debate_path
+    fill_in 'debate_title', with: 'A test'
+    fill_in 'debate_description', with: 'This is <script>alert("an attack");</script>'
+    check 'debate_terms_of_service'
+
+    click_button 'Create Debate'
+
+    expect(page).to have_content 'Debate was successfully created.'
+    expect(page).to have_content 'A test'
+    expect(page).to have_content 'This is alert("an attack");'
+    expect(page.html).to_not include '<script>alert("an attack");</script>'
+  end
+
   scenario 'Update should not be posible if logged user is not the author' do
     debate = create(:debate)
     expect(debate).to be_editable

spec/lib/wysiwyg_sanitizer_spec.rb

@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+describe WYSIWYGSanitizer do
+
+  subject { described_class.new }
+
+  describe '#sanitize' do
+
+    it 'returns an html_safe string' do
+      expect(subject.sanitize('hello')).to be_html_safe
+    end
+
+    it 'allows basic html formatting' do
+      html = '<p>This is <strong>a paragraph</strong></p>'
+      expect(subject.sanitize(html)).to eq(html)
+    end
+
+    it 'filters out dangerous tags' do
+      html = '<p>This is <script>alert("dangerous");</script></p>'
+      expect(subject.sanitize(html)).to eq('<p>This is alert("dangerous");</p>')
+    end
+  end
+
+end

spec/models/debate_spec.rb

@@ -25,6 +25,12 @@ describe Debate do
     expect(@debate).to_not be_valid
   end
 
+  it "should sanitize the description" do
+    @debate.description = "<script>alert('danger');</script>"
+    @debate.valid?
+    expect(@debate.description).to eq("alert('danger');")
+  end
+
   it "should not be valid without accepting terms of service" do
     @debate.terms_of_service = nil
     expect(@debate).to_not be_valid