Commit 41d61cba authored by Alessio Caiazza's avatar Alessio Caiazza Committed by Felipe Artur

Merge branch 'security-11-1-48617-promoting-milestone' into 'security-11-1'

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

See merge request gitlab/gitlabhq!2443
parent ae8a7797
...@@ -112,7 +112,7 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -112,7 +112,7 @@ class Projects::LabelsController < Projects::ApplicationController
begin begin
return render_404 unless promote_service.execute(@label) 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| respond_to do |format|
format.html do format.html do
redirect_to(project_labels_path(@project), status: 303) redirect_to(project_labels_path(@project), status: 303)
...@@ -135,6 +135,15 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -135,6 +135,15 @@ class Projects::LabelsController < Projects::ApplicationController
end end
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 protected
def label_params def label_params
......
...@@ -76,8 +76,8 @@ class Projects::MilestonesController < Projects::ApplicationController ...@@ -76,8 +76,8 @@ class Projects::MilestonesController < Projects::ApplicationController
def promote def promote
promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone) 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)}\"><u>group milestone</u></a>.".html_safe
respond_to do |format| respond_to do |format|
format.html do format.html do
redirect_to project_milestones_path(project) redirect_to project_milestones_path(project)
...@@ -90,6 +90,15 @@ class Projects::MilestonesController < Projects::ApplicationController ...@@ -90,6 +90,15 @@ class Projects::MilestonesController < Projects::ApplicationController
redirect_to milestone, alert: error.message redirect_to milestone, alert: error.message
end end
def flash_notice_for(milestone, group)
notice = ''.html_safe
notice << milestone.title
notice << ' promoted to '
notice << view_context.link_to('<u>group milestone</u>'.html_safe, group_milestone_path(group, milestone.iid))
notice << '.'
notice
end
def destroy def destroy
return access_denied! unless can?(current_user, :admin_milestone, @project) return access_denied! unless can?(current_user, :admin_milestone, @project)
......
---
title: Escapes milestone and label's names on flash notice when promoting them
merge_request:
author:
type: fixed
...@@ -143,6 +143,14 @@ describe Projects::LabelsController do ...@@ -143,6 +143,14 @@ describe Projects::LabelsController do
expect(GroupLabel.find_by(title: promoted_label_name)).not_to be_nil expect(GroupLabel.find_by(title: promoted_label_name)).not_to be_nil
end 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 context 'service raising InvalidRecord' do
before do before do
expect_any_instance_of(Labels::PromoteService).to receive(:execute) do |label| expect_any_instance_of(Labels::PromoteService).to receive(:execute) do |label|
......
...@@ -127,6 +127,14 @@ describe Projects::MilestonesController do ...@@ -127,6 +127,14 @@ describe Projects::MilestonesController do
expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\"><u>group milestone</u></a>.") expect(flash[:notice]).to eq("#{milestone.title} promoted to <a href=\"#{group_milestone_path(project.group, milestone.iid)}\"><u>group milestone</u></a>.")
expect(response).to redirect_to(project_milestones_path(project)) expect(response).to redirect_to(project_milestones_path(project))
end 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)}\"><u>group milestone</u></a>.")
end
end end
context 'promotion fails' do context 'promotion fails' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment