Removed a number of uses of html_safe to ensure unsafe input can't ge…

…t through

app/helpers/home_helper.rb

@@ -27,7 +27,7 @@ def sort_by_duration
   #----------------------------------------------------------------------------
   def sort_by_users
     users = [[ "all_users", t(:option_all_users) ]] + @all_users.map do |user|
-      escaped = escape_javascript(user.full_name)
+      escaped = sanitize(user.full_name)
       [ escaped, escaped ]
     end
 

app/mailers/subscription_mailer.rb

@@ -3,6 +3,7 @@
 # Fat Free CRM is freely distributable under the terms of MIT license.
 # See MIT-LICENSE file or http://www.opensource.org/licenses/mit-license.php
 #------------------------------------------------------------------------------
+
 class SubscriptionMailer < ActionMailer::Base
 
   def comment_notification(user, comment)

app/models/polymorphic/task.rb

@@ -63,7 +63,7 @@ class Task < ActiveRecord::Base
     where('user_id = ? OR assigned_to = ?', user.id, user.id)
   }
 
-  # Show opportunities which either belong to the user and are unassigned, or are assigned to the user
+  # Show tasks which either belong to the user and are unassigned, or are assigned to the user
   scope :visible_on_dashboard, ->(user) {
     where('(user_id = :user_id AND assigned_to IS NULL) OR assigned_to = :user_id', :user_id => user.id).where('completed_at IS NULL')
   }

app/views/entities/_basic_search.html.haml

@@ -5,8 +5,7 @@
 %div{ :style => "margin: 0px 0px 6px 0px" }
   = text_field_tag('query', @current_query, :size => 32, :placeholder => "Search #{controller_name}")
   %span.sorting_options
-    -# sort_by_displaying: Sort {{models}} by {{field}} displaying first name {{position}} last name.
-    = t(:sort_by, :models => t(:"#{controller_name}_small"), :field => link_to(current_sort_by, "#", :id => :sort_by)).html_safe
+    = t(:sort_by, :field => link_to(h(current_sort_by), "#", :id => :sort_by)).html_safe
 
 :javascript
   var searchTimeout;

app/views/fields/_group_view.html.haml

@@ -4,4 +4,6 @@
     %tr
       - group.each do |field|
         = col(field.label, field.render_value(entity), (i == groups.size - 1) ? :last : nil)
-        = "<th class='last'></th><td class='last'></td>".html_safe if group.size == 1
+        - if group.size == 1
+          %th.last
+          %td.last

app/views/home/_activity.html.haml

@@ -1,4 +1,4 @@
-- user = link_to(h(activity.user.full_name), user_path(activity.user)) if activity.user
+- user = link_to(activity.user.full_name, user_path(activity.user)) if activity.user
 
 - subject = if (item = activity.related || activity.item)
   - if item.respond_to?(:full_name)
@@ -22,12 +22,14 @@
         - action = t('action_' + activity.event)
         - type = t('subject_' + activity.item_type.downcase)
         = link_to avatar_for(activity.user, :size => :thumb), user_path(activity.user)
-        = t("activity_text", :user => user, :action => action, :type => type, :subject => subject, :default => "#{user} #{action} #{type} #{subject}").html_safe # locales optionally can change the word order for activities using 'activity_text'
+        = t("activity_text", :user => h(user), :action => h(action), :type => h(type), :subject => h(subject), :default => "#{h user} #{h action} #{h type} #{h subject}").html_safe # locales optionally can change the word order for activities using 'activity_text'
         - if ((item = activity.item).class == Comment) and Setting.comments_visible_on_dashboard
           = t('action_create_comment', :comment => truncate(h(item.comment), :length => 90)).html_safe
       - else
         - type = t(activity.item_type.downcase)
         - if item.respond_to? :email
           = avatar_for(item, :size => "16x16")
-        = "#{type} #{subject}:".html_safe
-        = auto_link(t(activity.event)).html_safe
+        = type
+        = subject
+        = ":"
+        = auto_link(t(activity.event))

app/views/home/_options.html.haml

@@ -3,7 +3,7 @@
   = hidden_field_tag "account[user_id]", current_user.id
 
   -# activity_options: Show %{models} %{action_type} performed by %{user} in the past %{period}
-  = t(:activity_options, :models => link_to(t(@asset).singularize.downcase, "#", :id => :asset), :action_type => link_to(t(@action + "_past_participle").downcase, "#", :id => :event), :user => link_to(t(@user), "#", :id => :user), :period => link_to(t(@duration).downcase, "#", :id => :duration)).html_safe
+  = t(:activity_options, :models => link_to(t(h(@asset)).singularize.downcase, "#", :id => :asset), :action_type => link_to(t(h(@action) + "_past_participle").downcase, "#", :id => :event), :user => link_to(t(h(@user)), "#", :id => :user), :period => link_to(t(h(@duration)).downcase, "#", :id => :duration)).html_safe
 
 %script
   = render "assets_menu"

app/views/home/_task.html.haml

@@ -6,10 +6,10 @@
   .indent
     %label{ :id => dom_id(task, :name) }
       - if task.user.id != current_user.id
-        = t(:task_from, link_to(task.user.full_name, user_path(task.user))).html_safe << ':'
-      = link_to(h(task.name), tasks_path)
+        = t(:task_from, link_to(h(task.user.full_name), user_path(task.user))).html_safe << ':'
+      = link_to(task.name, tasks_path)
       - if task.asset_id?
-        == #{t :related} #{link_to(h(task.asset.name), polymorphic_url(task.asset))}
+        == #{t :related} #{link_to(task.asset.name, polymorphic_url(task.asset))}
       &ndash;
     %tt
       - if task.bucket == "due_asap"
@@ -45,4 +45,4 @@
 
     - unless task.background_info.blank?
       %div
-        %dt= h(task.background_info)
+        %dt= auto_link(simple_format task.background_info)

app/views/home/_users_menu.html.haml

@@ -4,5 +4,5 @@
     fade      : 500,
     appear    : 500,
     width     : 180,
-    menu_items: [ #{sort_by_users.join(",")} ]
+    menu_items: [ #{sort_by_users.join(',')} ]
   });

app/views/leads/_sidebar_show.html.haml

@@ -54,7 +54,7 @@
 
   - unless @lead.background_info.blank?
     .caption #{t :background_info}
-    = auto_link(simple_format @lead.background_info).html_safe
+    = auto_link(simple_format @lead.background_info)
 
   = render "fields/sidebar_show", :asset => @lead
 

app/views/opportunities/_sidebar_show.html.haml

@@ -50,10 +50,10 @@
 
   - unless @opportunity.background_info.blank?
     .caption #{t :background_info}
-    = auto_link(simple_format @opportunity.background_info).html_safe
+    = auto_link(simple_format @opportunity.background_info)
 
   = render "fields/sidebar_show", :asset => @opportunity
-  
+
   - if @opportunity.tag_list.present?
     %dt
       .tags= tags_for_index(@opportunity)

app/views/shared/_comment.html.haml

@@ -7,5 +7,5 @@
 
   .indentslim
     = link_to comment.user.full_name, user_path(comment.user)
-    %tt= timeago(comment.created_at).html_safe
-    %dt= auto_link(simple_format comment.comment).html_safe
+    %tt= timeago(comment.created_at)
+    %dt= auto_link(simple_format comment.comment)

app/views/subscription_mailer/comment_notification.text.erb

@@ -1,6 +1,6 @@
 <%= I18n.t('comment_notification.intro', :user_full_name => @user.full_name, :entity_type => @entity_type, :entity_name => @entity_name) %>
 
-<%= @comment.comment.html_safe %>
+<%= sanitize(@comment.comment) %>
 
 
 --

app/views/tasks/_completed.html.haml

@@ -19,7 +19,7 @@
     %tt
       %span.cool
         -# :task_completed_by: "completed {{time_ago}} ago by {{user}}"
-        = t(:task_completed_by, :time_ago => distance_of_time_in_words(completed.completed_at, Time.now), :date => l(completed.completed_at, :format => :long), :user => ((completed.completor.id != current_user.id) ? link_to(completed.completor.full_name, user_path(completed.completor)) : t(:me))).html_safe
+        = t(:task_completed_by, :time_ago => distance_of_time_in_words(completed.completed_at, Time.now), :date => l(completed.completed_at, :format => :long), :user => ((completed.completor.id != current_user.id) ? link_to(h(completed.completor.full_name), user_path(completed.completor)) : t(:me))).html_safe
     - unless completed.background_info.blank?
       %div
-        %dt= h(completed.background_info)
+        %dt= completed.background_info

app/views/tasks/_pending.html.haml

@@ -14,10 +14,11 @@
   .indentwide
     %label{ :id => dom_id(pending, :name) }
       - if pending.user.id != current_user.id
-        = t(:task_from, link_to(pending.user.full_name, user_path(pending.user))).html_safe << ':'
+        = t(:task_from, link_to(h(pending.user.full_name), user_path(pending.user))).html_safe << ':'
       = auto_link h(pending.name)
       - if pending.asset_id?
-        == #{t :related} #{link_to(h(pending.asset.name), polymorphic_url(pending.asset))}
+        =t :related
+        = link_to(h(pending.asset.name), polymorphic_url(pending.asset))
       &ndash;
     %tt
       - if pending.bucket == "due_asap"

app/views/tasks/_selector.html.haml

@@ -1,8 +1,8 @@
 %table{ :cellpadding => 3, :cellspacing => 0, :style => "width: 100%; margin: 2px 0px 10px 0px" }
   %tr
     %td{ :class => "selector left " + (@view == "pending" ? "on" : "off"), :width => "33%", :onclick => %Q(document.location.href='#{tasks_path}') }
-      #{t(:pending_tab).html_safe}
+      = t :pending_tab
     %td{ :class => "selector " + (@view == "assigned" ? "on" : "off"), :width => "33%", :onclick => %Q(document.location.href='#{url_for(:action => :index, :view => "assigned")}') }
-      #{t :assigned_tab}
+      = t :assigned_tab
     %td{ :class => "selector right " + (@view == "completed" ? "on" : "off"), :width => "33%", :onclick => %Q(document.location.href='#{url_for(:action => :index, :view => "completed")}') }
-      #{t :completed_tab}
+      = t :completed_tab

app/views/versions/_version.html.haml

@@ -9,8 +9,8 @@
     - if t('version.' + version.event, :default => 0) != 0
       = t('version.' + version.event, :item => item_type, :by => user_name, :default => version.event)
     - else
-      = auto_link(version.event).html_safe
-    %small= timeago(version.created_at).html_safe
+      = auto_link(version.event)
+    %small= timeago(version.created_at)
     %tt
       - version.changeset.each do |attr_name, change|
         - label, first, second = parse_version(attr_name, change)