Merge branch 'security-10-8-48617-promoting-milestone' into 'security-10-8'

[10.8] Escapes milestone title shown in flash message when promoting a milestone

See merge request gitlab/gitlabhq!2441

5 files changed, 42 insertions, 3 deletions

diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb
index 91016f6494e..83216c99561 100644
--- a/app/controllers/projects/labels_controller.rb
+++ b/app/controllers/projects/labels_controller.rb
@@ -112,7 +112,7 @@ class Projects::LabelsController < Projects::ApplicationController
     begin
       return render_404 unless promote_service.execute(@label)
 
-      flash[:notice] = "#{@label.title} promoted to <a href=\"#{group_labels_path(@project.group)}\">group label</a>.".html_safe
+      flash[:notice] = flash_notice_for(@label, @project.group)
       respond_to do |format|
         format.html do
           redirect_to(project_labels_path(@project), status: 303)
@@ -135,6 +135,15 @@ class Projects::LabelsController < Projects::ApplicationController
     end
   end
 
+  def flash_notice_for(label, group)
+    notice = ''.html_safe
+    notice << label.title
+    notice << ' promoted to '
+    notice << view_context.link_to('<u>group label</u>'.html_safe, group_labels_path(group))
+    notice << '.'
+    notice
+  end
+
   protected
 
   def label_params
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index c5a044541f1..99bd7eb25da 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -75,8 +75,8 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   def promote
     promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
+    flash[:notice] = flash_notice_for(promoted_milestone, project.group)
 
-    flash[:notice] = "#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, promoted_milestone.iid)}\">group milestone</a>.".html_safe
     respond_to do |format|
       format.html do
         redirect_to project_milestones_path(project)
@@ -89,6 +89,15 @@ class Projects::MilestonesController < Projects::ApplicationController
     redirect_to milestone, alert: error.message
   end
 
+  def flash_notice_for(milestone, group)
+    notice = ''.html_safe
+    notice << milestone.title
+    notice << ' promoted to '
+    notice << view_context.link_to('group milestone'.html_safe, group_milestone_path(group, milestone.iid))
+    notice << '.'
+    notice
+  end
+
   def destroy
     return access_denied! unless can?(current_user, :admin_milestone, @project)
 
diff --git a/changelogs/unreleased/48617-promoting-milestone.yml b/changelogs/unreleased/48617-promoting-milestone.yml
new file mode 100644
index 00000000000..7fbc15051cf
--- /dev/null
+++ b/changelogs/unreleased/48617-promoting-milestone.yml
@@ -0,0 +1,5 @@
+---
+title: Escapes milestone and label's names on flash notice when promoting them
+merge_request:
+author:
+type: fixed
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
index 452d7e23983..88955d879c9 100644
--- a/spec/controllers/projects/labels_controller_spec.rb
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -143,6 +143,14 @@ describe Projects::LabelsController do
         expect(GroupLabel.find_by(title: promoted_label_name)).not_to be_nil
       end
 
+      it 'renders label name without parsing it as HTML' do
+        label_1.update!(name: 'CCC&lt;img src=x onerror=alert(document.domain)&gt;')
+
+        post :promote, namespace_id: project.namespace.to_param, project_id: project, id: label_1.to_param
+
+        expect(flash[:notice]).to eq("CCC&lt;img src=x onerror=alert(document.domain)&gt; promoted to <a href=\"#{group_labels_path(project.group)}\"><u>group label</u></a>.")
+      end
+
       context 'service raising InvalidRecord' do
         before do
           expect_any_instance_of(Labels::PromoteService).to receive(:execute) do |label|
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 548c5ef36e7..0ea64600f38 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -107,8 +107,16 @@ describe Projects::MilestonesController do
       it 'shows group milestone' do
         post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
 
-        expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\">group milestone</a>.")
         expect(response).to redirect_to(project_milestones_path(project))
+        expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\">group milestone</a>.")
+      end
+
+      it 'renders milestone name without parsing it as HTML' do
+        milestone.update!(name: 'CCC&lt;img src=x onerror=alert(document.domain)&gt;')
+
+        post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
+
+        expect(flash[:notice]).to eq("CCC promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\">group milestone</a>.")
       end
     end