various modifications to prevent xss

- validation of names and labels against /^[\w\s\'\-]*$/i
- html entities encoding

git-svn-id: http://redmine.rubyforge.org/svn/trunk@99 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/helpers/issues_helper.rb

@@ -56,9 +56,9 @@ module IssuesHelper
     
     unless no_html
       label = content_tag('strong', label)
-      old_value = content_tag("i", old_value) if old_value
+      old_value = content_tag("i", h(old_value)) if old_value
-      old_value = content_tag("strike", old_value) if old_value and !value
+      old_value = content_tag("strike", h(old_value)) if old_value and !value
-      value = content_tag("i", value) if value
+      value = content_tag("i", h(value)) if value
     end
     
     if value

app/models/custom_field.rb

@@ -28,6 +28,7 @@ class CustomField < ActiveRecord::Base
 
   validates_presence_of :name, :field_format
   validates_uniqueness_of :name
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
   validates_inclusion_of :field_format, :in => FIELD_FORMATS.keys
   validates_presence_of :possible_values, :if => Proc.new { |field| field.field_format == "list" }
 

app/models/enumeration.rb

@@ -18,8 +18,9 @@
 class Enumeration < ActiveRecord::Base
   before_destroy :check_integrity
   
-	validates_presence_of :opt, :name
+  validates_presence_of :opt, :name
-	validates_uniqueness_of :name, :scope => [:opt]
+  validates_uniqueness_of :name, :scope => [:opt]
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
 	
 	OPTIONS = {
 	  "IPRI" => :enumeration_issue_priorities,

app/models/issue_status.rb

@@ -21,6 +21,7 @@ class IssueStatus < ActiveRecord::Base
 
   validates_presence_of :name
   validates_uniqueness_of :name
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
   validates_length_of :html_color, :is => 6
   validates_format_of :html_color, :with => /^[a-f0-9]*$/i
 

app/models/project.rb

@@ -31,6 +31,7 @@ class Project < ActiveRecord::Base
   validates_presence_of :name, :description
   validates_uniqueness_of :name
   validates_associated :custom_values, :on => :update
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
 
   # returns 5 last created projects
   def self.latest

app/models/role.rb

@@ -23,6 +23,7 @@ class Role < ActiveRecord::Base
 
   validates_presence_of :name
   validates_uniqueness_of :name
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
 
 private
   def check_integrity

app/models/tracker.rb

@@ -23,7 +23,8 @@ class Tracker < ActiveRecord::Base
 
   validates_presence_of :name
   validates_uniqueness_of :name
-  
+  validates_format_of :name, :with => /^[\w\s\'\-]*$/i
+
 private
   def check_integrity
     raise "Can't delete tracker" if Issue.find(:first, :conditions => ["tracker_id=?", self.id])

app/models/user.rb

@@ -32,7 +32,8 @@ class User < ActiveRecord::Base
   validates_presence_of :login, :firstname, :lastname, :mail
   validates_uniqueness_of :login, :mail	
   # Login must contain lettres, numbers, underscores only
-  validates_format_of :login, :with => /^[a-z0-9_]+$/i
+  validates_format_of :firstname, :lastname, :with => /^[\w\s\'\-]*$/i
+  validates_format_of :login, :with => /^[a-z0-9_\-@\.]+$/i
   validates_format_of :mail, :with => /^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i
   # Password length between 4 and 12
   validates_length_of :password, :in => 4..12, :allow_nil => true

app/views/admin/projects.rhtml

@@ -17,7 +17,7 @@
 <% for project in @projects %>
   <tr class="<%= cycle("odd", "even") %>">
 	<td><%= link_to project.name, :controller => 'projects', :action => 'settings', :id => project %>
-	<td><%= project.description %>
+	<td><%=h project.description %>
 	<td align="center"><%= image_tag 'true' if project.is_public? %>
 	<td align="center"><%= project.projects_count %>
 	<td align="center"><%= format_date(project.created_on) %>

app/views/documents/_document.rhtml

@@ -0,0 +1,3 @@
+<p><%= link_to h(document.title), :controller => 'documents', :action => 'show', :id => document %><br />
+<% unless document.description.empty? %><%=h truncate document.description, 250 %><br /><% end %>
+<em><%= format_time(document.created_on) %></em></p>

app/views/issues/_history.rhtml

@@ -6,6 +6,6 @@
 	<% end %>
 	</ul>
 	<% if journal.notes? %>
-    	<%= simple_format auto_link journal.notes %>
+    	<%= simple_format auto_link h(journal.notes) %>
 	<% end %>
 <% end %>

app/views/issues/_list_simple.rhtml

@@ -15,7 +15,7 @@
 			<td><p class="small"><%= issue.project.name %> - <%= issue.tracker.name %><br />
                 <%= issue.status.name %> - <%= format_time(issue.updated_on) %></p></td>
 			<td>
-                <p class="small"><%= link_to issue.subject, :controller => 'issues', :action => 'show', :id => issue %></p>
+                <p class="small"><%= link_to h(issue.subject), :controller => 'issues', :action => 'show', :id => issue %></p>
             </td>
 		</tr>
 		<% end %>

app/views/issues/change_status.rhtml

@@ -1,4 +1,4 @@
-<h2><%=l(:label_issue)%> #<%= @issue.id %>: <%= @issue.subject %></h2>
+<h2><%=l(:label_issue)%> #<%= @issue.id %>: <%=h @issue.subject %></h2>
 
 <%= error_messages_for 'issue' %>
 <%= start_form_tag({:action => 'change_status', :id => @issue}, :class => "tabular") %>

app/views/issues/show.rhtml

@@ -2,7 +2,7 @@
 <%= l(:label_export_to) %><%= link_to 'PDF', {:action => 'export_pdf', :id => @issue}, :class => 'pic picPdf' %>
 </div>
 
-<h2><%= @issue.tracker.name %> #<%= @issue.id %> - <%= @issue.subject %></h2>
+<h2><%= @issue.tracker.name %> #<%= @issue.id %> - <%=h @issue.subject %></h2>
 
 <div class="box">
 <table width="100%">
@@ -12,7 +12,7 @@
 </tr>
 <tr>
     <td><b><%=l(:field_assigned_to)%> :</b></td><td><%= @issue.assigned_to ? @issue.assigned_to.name : "-" %></td>
-    <td><b><%=l(:field_category)%> :</b></td><td><%= @issue.category ? @issue.category.name : "-" %></td>
+    <td><b><%=l(:field_category)%> :</b></td><td><%=h @issue.category ? @issue.category.name : "-" %></td>
 </tr>
 <tr>
     <td><b><%=l(:field_author)%> :</b></td><td><%= link_to_user @issue.author %></td>
@@ -29,7 +29,7 @@
 <tr>
 <% n = 0
 for custom_value in @custom_values %>
-    <td><b><%= custom_value.custom_field.name %> :</b></td><td><%= show_value custom_value %></td>
+    <td><b><%= custom_value.custom_field.name %> :</b></td><td><%=h show_value custom_value %></td>
 <% n = n + 1
    if (n > 1) 
         n = 0 %>

app/views/my/blocks/_calendar.rhtml

@@ -34,7 +34,7 @@ while day <= @date_to
 		elsif day == i.due_date
 		    image_tag('arrow_to') 
 		end %>
-		<small><%= link_to "#{i.tracker.name} ##{i.id}", :controller => 'issues', :action => 'show', :id => i %>: <%= i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small><br />
+		<small><%= link_to "#{i.tracker.name} ##{i.id}", :controller => 'issues', :action => 'show', :id => i %>: <%=h i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small><br />
 	<% end %>
     </td>
 	<%= '</tr><tr height="100">' if day.cwday >= 7 and day!=@date_to %>

app/views/my/blocks/_documents.rhtml

@@ -1,15 +1,7 @@
 <h3><%=l(:label_document_plural)%></h3>
 
-<ul>
+<%= render(:partial => 'documents/document',
-<% for document in Document.find :all,
+           :collection => Document.find(:all,
                          :limit => 10,
                          :conditions => "documents.project_id in (#{@user.projects.collect{|m| m.id}.join(',')})",
-                         :include => [:project] %>
+                         :include => [:project])) unless @user.projects.empty? %>
-	<li>
-	<b><%= link_to document.title, :controller => 'documents', :action => 'show', :id => document %></b>
-	<br />
-	<%= truncate document.description, 150 %><br />
-	<em><%= format_time(document.created_on) %></em><br />&nbsp;
-	</li>
-<% end unless @user.projects.empty? %>
-</ul>

app/views/my/blocks/_latest_news.rhtml

@@ -1,13 +1,7 @@
 <h3><%=l(:label_news_latest)%></h3>
 
-<ul>
+<%= render (:partial => 'news/news', 
-<% for news in News.find :all,
+            :collection => News.find(:all,
-                         :limit => 10,
+                                     :limit => 10,
-                         :conditions => "news.project_id in (#{@user.projects.collect{|m| m.id}.join(',')})",
+                                     :conditions => "news.project_id in (#{@user.projects.collect{|m| m.id}.join(',')})",
-                         :include => [:project, :author] %>
+                                     :include => [:project, :author])) unless @user.projects.empty? %>
-	<li><%= link_to news.title, :controller => 'news', :action => 'show', :id => news %><br />
-	<% unless news.summary.empty? %><%= news.summary %><br /><% end %>
-	<em><%= news.author.name %>, <%= format_time(news.created_on) %></em><br />&nbsp;
-	</li>
-<% end unless @user.projects.empty? %>
-</ul>

app/views/my/page.rhtml

@@ -1,9 +1,9 @@
-<h2><%=l(:label_my_page)%></h2>
+<div class="contextual">
-
+    <%= link_to l(:label_personalize_page), :action => 'page_layout' %>
-<div class="topright">
-    <small><%= link_to l(:label_personalize_page), :action => 'page_layout' %></small>
 </div>
 
+<h2><%=l(:label_my_page)%></h2>
+
 <div id="list-top">
 	<% @blocks['top'].each do |b| %>
 	<div class="mypage-box">        

app/views/my/page_layout.rhtml

@@ -34,11 +34,10 @@ function removeBlock(block) {
 
 </script>
 
-<div style="float:right;">
+<div class="contextual">
+<span id="indicator" style="display:none"><%= image_tag "loading.gif", :align => "absmiddle" %></span>
 <%= start_form_tag({:action => "add_block"}, :id => "block-form") %>
-
+<%= select_tag 'block', "<option></option>" + options_for_select(@block_options), :id => "block-select" %>
-<%= select_tag 'block', "<option></option>" + options_for_select(@block_options), :id => "block-select", :class => "select-small" %>
-<small>
 <%= link_to_remote l(:button_add),
             :url => { :action => "add_block" },
             :with => "Form.serialize('block-form')",
@@ -48,16 +47,9 @@ function removeBlock(block) {
             :loading => "Element.show('indicator')",
             :loaded => "Element.hide('indicator')"
              %>
-</small>
+<%= end_form_tag %> |
-<%= end_form_tag %> 
-<small>|
 <%= link_to l(:button_save), :action => 'page_layout_save' %> |
 <%= link_to l(:button_cancel), :action => 'page' %>
-</small>
-</div>
-
-<div style="float:right;margin-right:20px;">
-<span id="indicator" style="display:none"><%= image_tag "loading.gif" %></span>
 </div>
 
 <h2><%=l(:label_my_page)%></h2>

app/views/news/_news.rhtml

@@ -0,0 +1,4 @@
+<p><%= link_to h(news.title), :controller => 'news', :action => 'show', :id => news %><br />
+<% unless news.summary.empty? %><%=h news.summary %><br /><% end %>
+<em><%= news.author.name %>, <%= format_time(news.created_on) %></em><br />
+<%= news.comments_count %> <%= lwr(:label_comment, news.comments_count).downcase %><br /></p>

app/views/news/show.rhtml

@@ -3,9 +3,9 @@
 <%= link_to_if_authorized l(:button_delete), {:controller => 'news', :action => 'destroy', :id => @news}, :confirm => l(:text_are_you_sure), :post => true, :class => 'pic picDelete' %>
 </div>
 
-<h2><%= @news.title %></h2>
+<h2><%=h @news.title %></h2>
 
-<p><em><%= @news.summary %><br />
+<p><em><%=h @news.summary %><br />
 <%= @news.author.display_name %>, <%= format_time(@news.created_on) %></em></p>
 <br />
 <%= textilizable auto_link @news.description %>

app/views/projects/activity.rhtml

@@ -18,20 +18,20 @@
   <% @events_by_day[day].sort {|x,y| y.created_on <=> x.created_on }.each do |e| %>
     <li><p>
     <% if e.is_a? Issue %>
-      <%= e.created_on.strftime("%H:%M") %> <%= link_to "#{e.tracker.name} ##{e.id}", :controller => 'issues', :action => 'show', :id => e %> (<%= e.status.name %>): <%= e.subject %><br />
+      <%= e.created_on.strftime("%H:%M") %> <%= link_to "#{e.tracker.name} ##{e.id}", :controller => 'issues', :action => 'show', :id => e %> (<%= e.status.name %>): <%=h e.subject %><br />
           <i><%= e.author.name %></i>
     <% elsif e.is_a? News %>
-      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_news)%>: <%= link_to e.title, :controller => 'news', :action => 'show', :id => e %><br />
+      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_news)%>: <%= link_to h(e.title), :controller => 'news', :action => 'show', :id => e %><br />
-          <% unless e.summary.empty? %><%= e.summary %><br /><% end %>
+          <% unless e.summary.empty? %><%=h e.summary %><br /><% end %>
           <i><%= e.author.name %></i>
     <% elsif (e.is_a? Attachment) and (e.container.is_a? Version) %>
-      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_attachment)%> (<%= e.container.name %>): <%= link_to e.filename, :controller => 'projects', :action => 'list_files', :id => @project %><br />
+      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_attachment)%> (<%=h e.container.name %>): <%= link_to e.filename, :controller => 'projects', :action => 'list_files', :id => @project %><br />
         <i><%= e.author.name %></i>
     <% elsif (e.is_a? Attachment) and (e.container.is_a? Document) %>
-      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_attachment)%>: <%= e.filename %> (<%= link_to e.container.title, :controller => 'documents', :action => 'show', :id => e.container %>)<br />
+      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_attachment)%>: <%= e.filename %> (<%= link_to h(e.container.title), :controller => 'documents', :action => 'show', :id => e.container %>)<br />
         <i><%= e.author.name %></i>
     <% elsif e.is_a? Document %>
-      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_document)%>: <%= link_to e.title, :controller => 'documents', :action => 'show', :id => e %><br />
+      <%= e.created_on.strftime("%H:%M") %> <%=l(:label_document)%>: <%= link_to h(e.title), :controller => 'documents', :action => 'show', :id => e %><br />
     <% end %>
     </p></li>
   

app/views/projects/calendar.rhtml

@@ -50,7 +50,7 @@ while day <= @date_to
 		elsif day == i.due_date
 		    image_tag('arrow_to') 
 		end %>
-		<small><%= link_to "#{i.tracker.name} ##{i.id}", :controller => 'issues', :action => 'show', :id => i %>: <%= i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small><br />
+		<small><%= link_to "#{i.tracker.name} ##{i.id}", :controller => 'issues', :action => 'show', :id => i %>: <%=h i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small><br />
 	<% end %>
     </td>
 	<%= '</tr><tr height="100">' if day.cwday >= 7 and day!=@date_to %>

app/views/projects/changelog.rhtml

@@ -23,6 +23,6 @@
     <ul>
     <% ver_id = issue.fixed_version_id
     end %>
-    <li><%= link_to issue.long_id, :controller => 'issues', :action => 'show', :id => issue %> [<%= issue.tracker.name %>]: <%= issue.subject %></li>
+    <li><%= link_to issue.long_id, :controller => 'issues', :action => 'show', :id => issue %> [<%= issue.tracker.name %>]: <%=h issue.subject %></li>
 <% end %>
 </div>

app/views/projects/export_issues_pdf.rfpdf

@@ -1,10 +1,9 @@
 <%	pdf=IfpdfHelper::IFPDF.new
 	pdf.AliasNbPages
 	pdf.footer_date = format_date(Date.today)
-	pdf.AddPage
 	@issues.each {|i| 
-		render :partial => 'issues/pdf', :locals => { :pdf => pdf, :issue => i }
 		pdf.AddPage
+		render :partial => 'issues/pdf', :locals => { :pdf => pdf, :issue => i }
 	}	
 %>
 

app/views/projects/gantt.rhtml

@@ -103,7 +103,7 @@ top = headers_heigth + 8
 @issues.each do |i| %>
 	<div style="position: absolute;line-height:1em;height:16px;top:<%= top %>px;left:4px;width:<%= subject_width - 5 %>px;overflow:hidden;">
 	<small><%= link_to "#{i.tracker.name} ##{i.id}", :controller => 'issues', :action => 'show', :id => i %>:
-	<%= i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small>
+	<%=h i.subject.sub(/^(.{30}[^\s]*\s).*$/, '\1 (...)') %></small>
 	</div>
 <% top = top + 20
 end %>

app/views/projects/list.rhtml

@@ -10,7 +10,7 @@
 <% for project in @projects %>
   <tr class="<%= cycle("odd", "even") %>">
 	<td><%= link_to project.name, :action => 'show', :id => project %>
-	<td><%= project.description %>
+	<td><%=h project.description %>
 	<td align="center"><%= format_date(project.created_on) %>
   </tr>
 <% end %>

app/views/projects/list_documents.rhtml

@@ -8,16 +8,6 @@
 
 <% documents = @documents.group_by {|d| d.category } %>
 <% documents.each do |category, docs| %>
-<h3><%= category.name %></h3>
+    <h3><%= category.name %></h3>
-<ul>
+    <%= render :partial => 'documents/document', :collection => docs %>
-<% docs.each do |d| %>
-  <li>
-    <b><%= link_to d.title, :controller => 'documents', :action => 'show', :id => d %></b>
-    <br />
-    <%= truncate d.description, 250 %><br />
-    <em><%= format_time(d.created_on) %></em><br />&nbsp;
-  </li>		
-
-<% end %>
-</ul>
 <% end %>

app/views/projects/list_issues.rhtml

@@ -69,7 +69,7 @@
 		<td align="center"><%= link_to issue.long_id, :controller => 'issues', :action => 'show', :id => issue %></td>
 		<td align="center" style="font-weight:bold;color:#<%= issue.status.html_color %>;"><%= issue.status.name %></font></td>
 		<td align="center"><%= issue.tracker.name %></td>
-		<td><%= link_to issue.subject, :controller => 'issues', :action => 'show', :id => issue %></td>
+		<td><%= link_to h(issue.subject), :controller => 'issues', :action => 'show', :id => issue %></td>
 		<td align="center"><%= issue.author.display_name %></td>
 		<td align="center"><%= format_time(issue.created_on) %></td>
 		<td align="center"><%= format_time(issue.updated_on) %></td>

app/views/projects/list_news.rhtml

@@ -5,16 +5,5 @@
 <h2><%=l(:label_news_plural)%></h2>
 
 <% if @news.empty? %><p><i><%= l(:label_no_data) %></i></p><% end %>
-
+<%= render :partial => 'news/news', :collection => @news %>
-<ul>
-<% for news in @news %>
-	<li><%= link_to news.title, :controller => 'news', :action => 'show', :id => news %><br />
-	<% unless news.summary.empty? %><%= news.summary %><br /><% end %>
-	<em><%= news.author.name %>, <%= format_time(news.created_on) %></em><br />
-	<%= news.comments_count %> <%= lwr(:label_comment, news.comments_count).downcase %><br />&nbsp;
-	</li>
-<% end %>
-</ul>
-
-
 <%= pagination_links_full @news_pages %>

app/views/projects/show.rhtml

@@ -1,13 +1,13 @@
 <h2><%=l(:label_overview)%></h2> 
 	
 <div class="splitcontentleft">
-	<%= simple_format(auto_link(@project.description)) %>	
+	<%= simple_format(auto_link(h @project.description)) %>	
 	<ul>
 	<% unless @project.homepage.empty? %><li><%=l(:field_homepage)%>: <%= auto_link @project.homepage %></li><% end %>
 	<li><%=l(:field_created_on)%>: <%= format_date(@project.created_on) %></li>
 	<% for custom_value in @custom_values %>
 	<% if !custom_value.value.empty? %>
-	   <li><%= custom_value.custom_field.name%>: <%= show_value(custom_value) %></li>
+	   <li><%= custom_value.custom_field.name%>: <%=h show_value(custom_value) %></li>
 	<% end %>
 	<% end %>
 	</ul>	
@@ -32,7 +32,7 @@
         <% end %>
         </ul>
     <% end %>
-    <center><small>[ <%= link_to l(:label_issue_view_all), :controller => 'projects', :action => 'list_issues', :id => @project, :set_filter => 1 %> ]</small></center>
+    <center><small><%= link_to l(:label_issue_view_all), :controller => 'projects', :action => 'list_issues', :id => @project, :set_filter => 1 %></small></center>
   </div>
 </div>
 
@@ -55,13 +55,8 @@
   
   <div class="box">
     <h3><%=l(:label_news_latest)%></h3>  
-    <% for news in @news %>
+    <%= render :partial => 'news/news', :collection => @news %>
-      <p><b><%= news.title %></b> <small>(<%= link_to_user news.author %> <%= format_time(news.created_on) %>)</small><br />
+    <center><small><%= link_to l(:label_news_view_all), :controller => 'projects', :action => 'list_news', :id => @project %></small></center>
-      <%= news.summary %>
-      <small>[<%= link_to l(:label_read), :controller => 'news', :action => 'show', :id => news %>]</small></p>
-      <hr />
-    <% end %>
-    <center><small>[ <%= link_to l(:label_news_view_all), :controller => 'projects', :action => 'list_news', :id => @project %> ]</small></center>
   </div>  
 </div>
 

app/views/users/_form.rhtml

@@ -11,7 +11,7 @@
 
 <% for @custom_value in @custom_values %>
 	<p><%= custom_field_tag_with_label @custom_value %></p>
-<% end %>
+<% end if @custom_values%>
 
 <p><%= f.check_box :admin %></p>
 <p><%= f.check_box :mail_notification %></p>

app/views/welcome/index.rhtml

@@ -4,14 +4,7 @@
   <% if $RDM_WELCOME_TEXT %><p><%= $RDM_WELCOME_TEXT %></p><br /><% end %>
   <div class="box">
 	<h3><%=l(:label_news_latest)%></h3>
-		<% for news in @news %>
+		<%= render :partial => 'news/news', :collection => @news %>
-			<p>
-			<b><%= news.title %></b> (<%= link_to_user news.author %> <%= format_time(news.created_on) %> - <%= news.project.name %>)<br />
-			<% unless news.summary.empty? %><%= news.summary %><br /><% end %>
-			[<%= link_to l(:label_read), :controller => 'news', :action => 'show', :id => news %>]
-			</p>
-			<hr />
-		<% end %>
 	</div>
 </div>
 
@@ -22,7 +15,7 @@
 		<% for project in @projects %>
 			<li>
 			<%= link_to project.name, :controller => 'projects', :action => 'show', :id => project %> (<%= format_time(project.created_on) %>)<br />
-			<%= project.description %>
+			<%=h project.description %>
 			</li>
 		<% end %>
 		</ul>

public/stylesheets/application.css

@@ -473,6 +473,9 @@ float: right;
 font-size: 0.8em;
 }
 
+.contextual select {
+font-size: 1em;
+}
 
 
 /***** CSS FORM ******/