From 31cf51f07a756a80d4c0a8bf3a03a3c7cb090dd6 Mon Sep 17 00:00:00 2001
From: David Gil <dgilperez@gmail.com>
Date: Thu, 10 Sep 2015 18:28:10 +0200
Subject: [PATCH] adds text_with_links helper and use that in any comment.body
 in views, adds test to check for malicious injections in comment body

---
 app/helpers/comments_helper.rb               |  2 +-
 app/helpers/text_with_links_helper.rb        |  9 +++++++++
 app/mailers/mailer.rb                        |  1 +
 app/models/comment.rb                        |  5 -----
 app/views/admin/comments/index.html.erb      |  2 +-
 app/views/admin/users/show.html.erb          |  2 +-
 app/views/comments/_comment.html.erb         | 14 +++++++-------
 app/views/mailer/comment.html.erb            |  2 +-
 app/views/mailer/reply.html.erb              |  2 +-
 app/views/moderation/comments/index.html.erb |  2 +-
 spec/features/comments_spec.rb               | 13 ++++++++++++-
 11 files changed, 35 insertions(+), 19 deletions(-)
 create mode 100644 app/helpers/text_with_links_helper.rb

diff --git a/app/helpers/comments_helper.rb b/app/helpers/comments_helper.rb
index 36952af7d..67c347ced 100644
--- a/app/helpers/comments_helper.rb
+++ b/app/helpers/comments_helper.rb
@@ -17,4 +17,4 @@ module CommentsHelper
     comments.select{|c| c.parent_id == parent.id}
   end
 
-end
\ No newline at end of file
+end
diff --git a/app/helpers/text_with_links_helper.rb b/app/helpers/text_with_links_helper.rb
new file mode 100644
index 000000000..32ada7c72
--- /dev/null
+++ b/app/helpers/text_with_links_helper.rb
@@ -0,0 +1,9 @@
+module TextWithLinksHelper
+
+  def text_with_links(text)
+    return unless text
+    sanitized = sanitize text, tags: %w(a), attributes: %w(href)
+    Rinku.auto_link(sanitized, :all, 'target="_blank" rel="nofollow"').html_safe
+  end
+
+end
diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb
index a3db690a8..15a8eb54a 100644
--- a/app/mailers/mailer.rb
+++ b/app/mailers/mailer.rb
@@ -1,4 +1,5 @@
 class Mailer < ApplicationMailer
+  helper :text_with_links
 
   def comment(comment)
     @comment = comment
diff --git a/app/models/comment.rb b/app/models/comment.rb
index 5836fa103..233736824 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -53,11 +53,6 @@ class Comment < ActiveRecord::Base
     self.user= author
   end
 
-  def body
-    unprocessed = super
-    unprocessed ? Rinku.auto_link(unprocessed, :all, 'target="_blank" rel="nofollow"').html_safe : unprocessed
-  end
-
   def total_votes
     cached_votes_total
   end
diff --git a/app/views/admin/comments/index.html.erb b/app/views/admin/comments/index.html.erb
index ec0fddc1a..ce4bfa293 100644
--- a/app/views/admin/comments/index.html.erb
+++ b/app/views/admin/comments/index.html.erb
@@ -9,7 +9,7 @@
     <li id="<%= dom_id(comment) %>">
       <div class="row">
         <div class="small-12 medium-8 column">
-          <%= comment.body %>
+          <%= text_with_links comment.body %>
           <%= link_to comment.commentable.title, comment.commentable %>
         </div>
         <div class="small-6 medium-4 column text-right">
diff --git a/app/views/admin/users/show.html.erb b/app/views/admin/users/show.html.erb
index 79a4b223e..6d98d4e38 100644
--- a/app/views/admin/users/show.html.erb
+++ b/app/views/admin/users/show.html.erb
@@ -31,7 +31,7 @@
   <li id="<%= dom_id(comment) %>">
       <div class="row">
         <div class="small-12 medium-10 column">
-          <%= comment.body %>
+          <%= text_with_links comment.body %>
         </div>
       </div>
     </li>
diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb
index 3997efdaf..25d7585f3 100644
--- a/app/views/comments/_comment.html.erb
+++ b/app/views/comments/_comment.html.erb
@@ -63,17 +63,17 @@
           </div>
 
           <% if comment.as_administrator? %>
-            <p class="comment-user is-admin"><%= comment.body %></p>
+            <p class="comment-user is-admin"><%= text_with_links comment.body %></p>
           <% elsif comment.as_moderator? %>
-            <p class="comment-user is-moderator"><%= comment.body %></p>
+            <p class="comment-user is-moderator"><%= text_with_links comment.body %></p>
           <% elsif comment.user.official? && comment.user_id == @commentable.author_id %>
-            <p class="comment-user level-<%= comment.user.official_level %> is-author"><%= comment.body %></p>
+            <p class="comment-user level-<%= comment.user.official_level %> is-author"><%= text_with_links comment.body %></p>
           <% elsif comment.user.official? %>
-            <p class="comment-user level-<%= comment.user.official_level %>"><%= comment.body %></p>
+            <p class="comment-user level-<%= comment.user.official_level %>"><%= text_with_links comment.body %></p>
           <% elsif comment.user_id == @commentable.author_id %>
-            <p class="comment-user is-author"><%= comment.body %></p>
+            <p class="comment-user is-author"><%= text_with_links comment.body %></p>
           <% else %>
-            <p class="comment-user"><%= comment.body %></p>
+            <p class="comment-user"><%= text_with_links comment.body %></p>
           <% end %>
           <span id="<%= dom_id(comment) %>_votes" class="comment-votes right">
             <%= render 'comments/votes', comment: comment %>
@@ -101,4 +101,4 @@
       </div>
     </div>
   </div>
-<% end %>
\ No newline at end of file
+<% end %>
diff --git a/app/views/mailer/comment.html.erb b/app/views/mailer/comment.html.erb
index 959b8e0a0..16ace7990 100644
--- a/app/views/mailer/comment.html.erb
+++ b/app/views/mailer/comment.html.erb
@@ -13,6 +13,6 @@
   </p>
 
   <p style="border-left: 2px solid #DEE0E3;font-family: 'Open Sans','Helvetica Neue',arial,sans-serif;font-size: 14px;font-style: italic;font-weight: normal;line-height: 24px;margin-left: 20px;padding: 10px;">
-    <%= @comment.body %>
+    <%= text_with_links @comment.body %>
   </p>
 </td>
diff --git a/app/views/mailer/reply.html.erb b/app/views/mailer/reply.html.erb
index 924234f02..d006e205a 100644
--- a/app/views/mailer/reply.html.erb
+++ b/app/views/mailer/reply.html.erb
@@ -13,6 +13,6 @@
   </p>
 
   <p style="border-left: 2px solid #DEE0E3;font-family: 'Open Sans','Helvetica Neue',arial,sans-serif;font-size: 14px;font-style: italic;font-weight: normal;line-height: 24px;margin-left: 20px;padding: 10px;">
-    <%= @reply.body %>
+    <%= text_with_links @reply.body %>
   </p>
 </td>
diff --git a/app/views/moderation/comments/index.html.erb b/app/views/moderation/comments/index.html.erb
index ec642077b..d18dca8e8 100644
--- a/app/views/moderation/comments/index.html.erb
+++ b/app/views/moderation/comments/index.html.erb
@@ -23,7 +23,7 @@
         <%= comment.commentable_type.constantize.model_name.human %>
         <span class="date"><%= l comment.updated_at.to_date %></span>
       </td>
-      <td><%= comment.body %></td>
+      <td><%= text_with_links comment.body %></td>
       <td class="text-center"><%= comment.flags_count %></td>
       <td>
         <%= link_to t("moderation.comments.index.hide"), hide_in_moderation_screen_moderation_comment_path(comment, request.query_parameters), method: :put, class: "delete" %>
diff --git a/spec/features/comments_spec.rb b/spec/features/comments_spec.rb
index b880ae284..41c1a5f31 100644
--- a/spec/features/comments_spec.rb
+++ b/spec/features/comments_spec.rb
@@ -20,7 +20,7 @@ feature 'Comments' do
     end
   end
 
-  scenario 'Autolinking' do
+  scenario 'Turns links into html links' do
     create :comment, commentable: debate, body: 'Built with http://rubyonrails.org/'
 
     visit debate_path(debate)
@@ -33,6 +33,17 @@ feature 'Comments' do
     end
   end
 
+  scenario 'Sanitizes comment body for security' do
+    create :comment, commentable: debate, body: "<script>alert('hola')</script> http://madrid.es"
+
+    visit debate_path(debate)
+
+    within first('.comment') do
+      expect(page).to have_content "alert('hola') http://madrid.es"
+      expect(page).to have_link('http://madrid.es', href: 'http://madrid.es')
+    end
+  end
+
   scenario 'Paginated comments' do
     per_page = 10
     (per_page + 2).times { create(:comment, commentable: debate)}