Prevent xss attack over group name. Added regex validation for group and team name

Dmitriy Zaporozhets
1 parent cfdf94fc
Showing 5 changed files with 22 additions and 6 deletions Show diff stats
app/helpers/application_helper.rb
app/helpers/projects_helper.rb
app/models/namespace.rb
app/models/user_team.rb
lib/gitlab/regex.rb
@@ -73,8 +73,8 @@ module ApplicationHelper
   def search_autocomplete_source
     projects = current_user.authorized_projects.map { |p| { label: "project: #{p.name_with_namespace}", url: project_path(p) } }
-    groups = current_user.authorized_groups.map { |group| { label: "group: #{group.name}", url: group_path(group) } }
-    teams = current_user.authorized_teams.map { |team| { label: "team: #{team.name}", url: team_path(team) } }
+    groups = current_user.authorized_groups.map { |group| { label: "group: #{simple_sanitize(group.name)}", url: group_path(group) } }
+    teams = current_user.authorized_teams.map { |team| { label: "team: #{simple_sanitize(team.name)}", url: team_path(team) } }
     default_nav = [
       { label: "My Profile", url: profile_path },
@@ -159,8 +159,13 @@ module ApplicationHelper
               alt: "Sign in with #{provider.to_s.titleize}")
   end
+  def simple_sanitize str
+    sanitize(str, tags: %w(a span))
+  end
+
   def image_url(source)
     root_url + path_to_image(source)
   end
+
   alias_method :url_to_image, :image_url
 end
@@ -56,7 +56,7 @@ module ProjectsHelper
   def project_title project
     if project.group
       content_tag :span do
-        link_to(project.group.name, group_path(project.group)) + " / " + project.name
+        link_to(simple_sanitize(project.group.name), group_path(project.group)) + " / " + project.name
       end
     else
       project.name
@@ -17,11 +17,15 @@ class Namespace &lt; ActiveRecord::Base
   has_many :projects, dependent: :destroy
   belongs_to :owner, class_name: "User"
-  validates :name, presence: true, uniqueness: true
+  validates :owner, presence: true
+  validates :name, presence: true, uniqueness: true,
+            length: { within: 0..255 },
+            format: { with: Gitlab::Regex.name_regex,
+                      message: "only letters, digits, spaces & '_' '-' '.' allowed." }
+
   validates :path, uniqueness: true, presence: true, length: { within: 1..255 },
             format: { with: Gitlab::Regex.path_regex,
                       message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" }
-  validates :owner, presence: true
   delegate :name, to: :owner, allow_nil: true, prefix: true
@@ -21,8 +21,11 @@ class UserTeam &lt; ActiveRecord::Base
   has_many :projects, through: :user_team_project_relationships
   has_many :members,  through: :user_team_user_relationships, source: :user
-  validates :name, presence: true, uniqueness: true
   validates :owner, presence: true
+  validates :name, presence: true, uniqueness: true,
+            length: { within: 0..255 },
+            format: { with: Gitlab::Regex.name_regex,
+                      message: "only letters, digits, spaces & '_' '-' '.' allowed." }
   validates :path, uniqueness: true, presence: true, length: { within: 1..255 },
             format: { with: Gitlab::Regex.path_regex,
                       message: "only letters, digits & '_' '-' '.' allowed. Letter should be first" }
@@ -10,6 +10,10 @@ module Gitlab
       /\A[a-zA-Z][a-zA-Z0-9_\-\. ]*\z/
     end
+    def name_regex
+      /\A[a-zA-Z0-9_\-\. ]*\z/
+    end
+
     def path_regex
       default_regex
     end
	@@ -56,7 +56,7 @@ module ProjectsHelper		@@ -56,7 +56,7 @@ module ProjectsHelper
56	def project_title project	56	def project_title project
57	if project.group	57	if project.group
58	content_tag :span do	58	content_tag :span do
59	- link_to(project.group.name, group_path(project.group)) + " / " + project.name	59	+ link_to(simple_sanitize(project.group.name), group_path(project.group)) + " / " + project.name
60	end	60	end
61	else	61	else
62	project.name	62	project.name