Merge pull request #1228 from ChrisBr/xss

[webui] Fix possible XSS attack on project title
openSUSE · Oct 12, 2015 · d78a29a · d78a29a
2 parents 6b39c9d + 9cc635b
commit d78a29a
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 8 deletions.
diff --git a/src/api/app/controllers/webui/project_controller.rb b/src/api/app/controllers/webui/project_controller.rb
@@ -48,7 +48,7 @@ def index
     @show_all = params[:show_all]
     projects = Project.all
     projects = projects.not_home unless @show_all
-    @projects = projects.pluck(:name, :title)
+    @projects = projects.select(:name, :title)
 
     atype = AttribType.find_by_namespace_and_name!('OBS', 'VeryImportantProject')
     @important_projects = Project.find_by_attribute_type(atype).where('name <> ?', 'deleted').pluck(:name, :title)

diff --git a/src/api/app/controllers/webui/user_controller.rb b/src/api/app/controllers/webui/user_controller.rb
@@ -66,7 +66,7 @@ def do_login
   end
 
   def show
-    @iprojects = @displayed_user.involved_projects.pluck(:name, :title)
+    @iprojects = @displayed_user.involved_projects.select(:name, :title)
     @ipackages = @displayed_user.involved_packages.joins(:project).pluck(:name, 'projects.name as pname')
     @owned = @displayed_user.owned_packages
 

diff --git a/src/api/app/helpers/webui/project_helper.rb b/src/api/app/helpers/webui/project_helper.rb
@@ -4,10 +4,6 @@ module Webui::ProjectHelper
 
   protected
 
-  def escape_project_list(arr)
-    arr.map { |p| "['#{p[0]}','#{escape_javascript(p[1])}']" }.join(",\n").html_safe
-  end
-
   def show_status_comment(comment, package, firstfail, comments_to_clear)
     status_comment_html = ''.html_safe
     if comment

diff --git a/src/api/app/helpers/webui/webui_helper.rb b/src/api/app/helpers/webui/webui_helper.rb
@@ -538,4 +538,17 @@ def can_register
     end
     return true
   end
+
+  def escape_project_list(projects)
+    # name and title are not html_safe
+    # because it's user input which we
+    # should never trust!!!
+    projects.map { |project|
+      "['".html_safe +
+      project.name +
+      "', '".html_safe +
+      escape_javascript(project.title) +
+      "']".html_safe
+    }.join(",\n").html_safe
+  end
 end
diff --git a/src/api/app/views/webui/user/show.html.erb b/src/api/app/views/webui/user/show.html.erb
@@ -93,8 +93,7 @@
     <% else %>
     <div id="projects_table_wrapper" data-url="<%= url_for(controller: 'project', action: 'show', project: 'REPLACEIT') %>">
     <% content_for :head_javascript do %>
-      var main_projects = [ <%= @iprojects.map {|p|
-                           "['#{p[0]}','#{escape_javascript(p[1])}']" }.join(",\n").html_safe %> ];
+      var main_projects = [ <%= escape_project_list(@iprojects) %> ];
       var excl_projects = [];
       <% end %>
       <% content_for :ready_function do %>