diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb
index 6b7dc73be88b380343a7bfe545214edd551a9f71..0b21ee4fccc99e2efd0aded93bdb4650a591d7d3 100644
--- a/app/helpers/labels_helper.rb
+++ b/app/helpers/labels_helper.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true
module LabelsHelper
+ include ActionView::Helpers::TagHelper
extend self
def show_label_issuables_link?(label, issuables_type, current_user: nil)
@@ -35,7 +36,7 @@ def show_label_issuables_link?(label, issuables_type, current_user: nil)
# # Customize link body with a block
# link_to_label(label) { "My Custom Label Text" }
#
- # Returns a String
+ # Returns a String containing HTML.
def link_to_label(label, type: :issue, tooltip: true, css_class: nil, &block)
link = label.filter_path(type: type)
@@ -46,6 +47,7 @@ def link_to_label(label, type: :issue, tooltip: true, css_class: nil, &block)
end
end
+ # Returns a String containing HTML.
def render_label(label, link: nil, tooltip: true, dataset: nil, tooltip_shows_title: false)
html = render_colored_label(label)
@@ -57,10 +59,11 @@ def render_label(label, link: nil, tooltip: true, dataset: nil, tooltip_shows_ti
wrap_label_html(html, label: label)
end
- def render_colored_label(label, suffix: '')
+ # Renders a label, with the given suffix HTML.
+ def render_colored_label(label, in_reference: nil)
render_label_text(
label.name,
- suffix: suffix,
+ in_reference: in_reference,
css_class: "gl-label-text #{label.text_color_class}",
bg_color: label.color
)
@@ -73,8 +76,9 @@ def wrap_label_html(label_html, label:)
%(#{label_html}).html_safe
end
+ # Returns a String containing text.
def label_tooltip_title(label, tooltip_shows_title: false)
- Sanitize.clean(tooltip_shows_title ? label.title : label.description)
+ tooltip_shows_title ? label.title : label.description
end
def suggested_colors
@@ -224,15 +228,13 @@ def render_label_link(label_html, link:, title:, dataset:)
link_to(label_html, link, class: classes.join(' '), data: dataset)
end
- def render_label_text(name, suffix: '', css_class: nil, bg_color: nil)
- <<~HTML.chomp.html_safe
- #{ERB::Util.html_escape_once(name)}#{suffix}
- HTML
+ def render_label_text(name, in_reference: nil, css_class: nil, bg_color: nil)
+ label_name_html = CGI.escapeHTML(name)
+ label_name_html << " " << content_tag(:i, "in #{in_reference}") if in_reference.present?
+
+ attrs = { "class" => css_class, "data-container" => "body", "data-html" => "true" }
+ attrs["style"] = "background-color: #{bg_color}" if bg_color
+ content_tag(:span, label_name_html.html_safe, **attrs)
end
end
diff --git a/ee/app/helpers/ee/labels_helper.rb b/ee/app/helpers/ee/labels_helper.rb
index 26b926ccc4d5f54c3196a909f34446670518942f..bc898baec84e72efbe0662c44050ff9f57ce20b9 100644
--- a/ee/app/helpers/ee/labels_helper.rb
+++ b/ee/app/helpers/ee/labels_helper.rb
@@ -8,7 +8,7 @@ module LabelsHelper
singleton_class.prepend self
end
- def render_colored_label(label, suffix: '')
+ def render_colored_label(label, in_reference: nil)
return super unless label.scoped_label?
render_label_text(
@@ -18,7 +18,7 @@ def render_colored_label(label, suffix: '')
) + render_label_text(
label.scoped_label_value,
css_class: "gl-label-text-scoped",
- suffix: suffix
+ in_reference: in_reference
)
end
@@ -31,11 +31,12 @@ def wrap_label_html(label_html, label:)
%(#{label_html}).html_safe
end
- def label_tooltip_title(label, tooltip_shows_title: false)
- tooltip = super
- tooltip = %(Scoped label
#{tooltip}) if label.scoped_label?
+ # Returns a String containing HTML.
+ def label_tooltip_title_html(label)
+ tooltip_html = CGI.escapeHTML(label_tooltip_title(label).to_s)
+ tooltip_html = %(Scoped label
#{tooltip_html}) if label.scoped_label?
- tooltip
+ tooltip_html
end
def label_dropdown_data(edit_context, opts = {})
diff --git a/ee/lib/ee/banzai/filter/references/label_reference_filter.rb b/ee/lib/ee/banzai/filter/references/label_reference_filter.rb
index 6cdab6d09621e090253d4ac4553d97ea0da5ee05..8bee34df542ad8275dc128fabbea23febf9fc3b4 100644
--- a/ee/lib/ee/banzai/filter/references/label_reference_filter.rb
+++ b/ee/lib/ee/banzai/filter/references/label_reference_filter.rb
@@ -9,11 +9,18 @@ module LabelReferenceFilter
override :data_attributes_for
def data_attributes_for(original, parent, object, link_content: false, link_reference: false)
- return super unless object.scoped_label?
-
- # Enabling HTML tooltips for scoped labels here.
+ # Causes the title (returned by `object_link_title` below) to be interpreted
+ # as HTML.
super.merge!(html: true)
end
+
+ # Returns a String containing HTML.
+ override :object_link_title
+ def object_link_title(object, _matches)
+ presenter = object.present(issuable_subject: project || group)
+
+ ::LabelsHelper.label_tooltip_title_html(presenter)
+ end
end
end
end
diff --git a/ee/spec/helpers/ee/labels_helper_spec.rb b/ee/spec/helpers/ee/labels_helper_spec.rb
index cdfb7927013b46df3ae1729257937d37882beb66..f9e6816a27bf0d6b005cdb0ed4d63152c3c3a476 100644
--- a/ee/spec/helpers/ee/labels_helper_spec.rb
+++ b/ee/spec/helpers/ee/labels_helper_spec.rb
@@ -123,4 +123,28 @@
}
end
end
+
+ describe '#label_tooltip_title_html' do
+ let(:description) { '
This is an image' }
+ let(:label_with_html_content) { create(:label, title: title, description: description) }
+ let(:tooltip) { label_tooltip_title_html(label_with_html_content) }
+
+ context 'when label is unscoped' do
+ let(:title) { 'test' }
+
+ it 'escapes HTML for display' do
+ expect(tooltip).to eq('<img src="example.png">This is an image</img>')
+ end
+ end
+
+ context 'when label is scoped' do
+ let(:title) { 'scope::test' }
+
+ it 'includes scoped label tag and escapes HTML for display' do
+ expect(tooltip).to eq(
+ "Scoped label
" \
+ '<img src="example.png">This is an image</img>')
+ end
+ end
+ end
end
diff --git a/ee/spec/lib/ee/banzai/filter/references/label_reference_filter_spec.rb b/ee/spec/lib/ee/banzai/filter/references/label_reference_filter_spec.rb
index 82ef964e7bd56c850b5e09b0b2a61879877e1167..1e87e7f467f313e01a3ba416bc829b9de8e40753 100644
--- a/ee/spec/lib/ee/banzai/filter/references/label_reference_filter_spec.rb
+++ b/ee/spec/lib/ee/banzai/filter/references/label_reference_filter_spec.rb
@@ -6,9 +6,9 @@
include FilterSpecHelper
let(:project) { create(:project, :public, name: 'sample-project') }
- let(:label) { create(:label, name: 'label', project: project) }
- let(:scoped_description) { 'xss <svg id="svgId"></svg>' }
- let(:scoped_label) { create(:label, name: 'key::value', project: project, description: scoped_description) }
+ let(:description) { 'xss <svg id="svgId"></svg>' }
+ let(:label) { create(:label, name: 'label', project: project, description: description) }
+ let(:scoped_label) { create(:label, name: 'key::value', project: project, description: description) }
context 'with scoped labels enabled' do
before do
@@ -27,17 +27,10 @@
end
it "doesn't unescape HTML in the label's title" do
- # Note that this is *flawed*: ">" from the input *text* is becoming ">" in the output *HTML*.
- # No XSS is possible but we are still corrupting the user's input, albeit less badly than before.
- # FIXME.
- expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('<svg id="svgId">')
-
- # The below is what *should* be the case, but LabelsHelper.label_tooltip_title is (incorrectly) sanitising
- # instead of safely escaping the output, and LabelsHelper.render_label_text is using html_escape_once (!!),
- # which is always bad!
- #
- # expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('xss <script>alert')
- # expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('&<a>lt;svg id="svgId">')
+ # The `title` attribute's DOM value is interpreted as HTML in EE, so we expect it contains the
+ # description escaped.
+ expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('xss <script>alert')
+ expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('&<a>lt;svg id="svgId">')
end
end
@@ -48,8 +41,13 @@
expect(doc.css('.gl-label .gl-label-text').map(&:text)).to eq([label.name])
end
- it 'renders non-HTML tooltips' do
- expect(doc.at_css('.gl-label a').attr('data-html')).to be_nil
+ it 'renders HTML tooltips' do
+ expect(doc.at_css('.gl-label a').attr('data-html')).to eq('true')
+ end
+
+ it "doesn't unescape HTML in the label's title" do
+ expect(doc.at_css('.gl-label a').attr('title')).to include('xss <script>alert')
+ expect(doc.at_css('.gl-label a').attr('title')).to include('&<a>lt;svg id="svgId">')
end
end
end
diff --git a/lib/banzai/filter/references/label_reference_filter.rb b/lib/banzai/filter/references/label_reference_filter.rb
index 07ad15247528d04daeabcbe1c13db0f1b8b6ff86..9b89c1ad84e70623357b30f6fcbd4fdaa8e2329c 100644
--- a/lib/banzai/filter/references/label_reference_filter.rb
+++ b/lib/banzai/filter/references/label_reference_filter.rb
@@ -118,19 +118,17 @@ def url_for_object(label, parent)
end
def object_link_text(object, matches)
- label_suffix = ''
+ reference = nil
parent = project || group
if matches[:absolute_path].blank? && (project || full_path_ref?(matches))
project_path = reference_cache.full_project_path(matches[:namespace], matches[:project], matches)
parent_from_ref = from_ref_cached(project_path)
reference = parent_from_ref.to_human_reference(parent)
-
- label_suffix = " in #{ERB::Util.html_escape(reference)}" if reference.present?
end
presenter = object.present(issuable_subject: parent)
- LabelsHelper.render_colored_label(presenter, suffix: label_suffix)
+ LabelsHelper.render_colored_label(presenter, in_reference: reference)
end
def wrap_link(link, label)
@@ -146,6 +144,7 @@ def reference_class(type, tooltip: true)
super + ' gl-link gl-label-link'
end
+ # Returns a String containing text.
def object_link_title(object, matches)
presenter = object.present(issuable_subject: project || group)
LabelsHelper.label_tooltip_title(presenter)
diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb
index ed3467eb96c37b86da5ce6c4f0a0471419afc3a6..82ebeaa63c3b7efe353929ab289d4cb239ca73e7 100644
--- a/spec/helpers/labels_helper_spec.rb
+++ b/spec/helpers/labels_helper_spec.rb
@@ -237,14 +237,14 @@
let(:label_with_html_content) { create(:label, title: 'test', description: html) }
context 'tooltip shows description' do
- it 'removes HTML' do
+ it 'leaves HTML untouched' do
tooltip = label_tooltip_title(label_with_html_content)
- expect(tooltip).to eq('This is an image')
+ expect(tooltip).to eq(html)
end
end
context 'tooltip shows title' do
- it 'shows title' do
+ it 'returns title' do
tooltip = label_tooltip_title(label_with_html_content, tooltip_shows_title: true)
expect(tooltip).to eq('test')
end
diff --git a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb
index f66af715a9900047f57dfb8145eed822aba57eed..fd25cc99bf62d214d69ec18602850578c1a8a232 100644
--- a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb
@@ -439,13 +439,23 @@
end
it 'has valid color' do
- expect(result.css('a span').first.attr('style')).to match(/background-color: #00ff00/)
+ expect(result.css('a span').first.attr('style')).to match('background-color: #00ff00')
end
it 'has valid link text' do
expect(result.css('a').first.text).to eq "#{label.name} in #{project2.full_name}"
end
+ it 'has correct HTML content' do
+ frag = Nokogiri::HTML.fragment("")
+ span = frag.children.first
+ span.content = "#{label.name} "
+ span << i = frag.document.create_element("i")
+ i.content = "in #{project2.full_name}"
+
+ expect(result.css('a').first.to_html).to include_html(span.inner_html)
+ end
+
it 'has valid text' do
expect(result.text).to eq "See #{label.name} in #{project2.full_name}"
end