Commit 79ebb144 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Only support HTML tooltips for scoped labels

This was unintentionally added and we only need it for scoped labels
where we add custom HTML to indicate that it's a scoped label.

This led to XSS issues on other references because we weren't escaping
those properly.
parent c7fc7839
---
title: Fix XSS in Markdown reference tooltips
merge_request:
author:
type: security
# frozen_string_literal: true
module EE
module Banzai
module Filter
module LabelReferenceFilter
extend ::Gitlab::Utils::Override
override :data_attributes_for
def data_attributes_for(text, parent, object, link_content: false, link_reference: false)
return super unless object.scoped_label?
# Enabling HTML tooltips for scoped labels here but we do not need to do any additional
# escaping because the label's tooltips are already stripped of dangerous HTML
super.merge!(
html: true
)
end
end
end
end
end
...@@ -14,16 +14,28 @@ RSpec.describe Banzai::Filter::LabelReferenceFilter do ...@@ -14,16 +14,28 @@ RSpec.describe Banzai::Filter::LabelReferenceFilter do
stub_licensed_features(scoped_labels: true) stub_licensed_features(scoped_labels: true)
end end
it 'renders scoped label with link to documentation' do context 'with a scoped label' do
doc = reference_filter("See #{scoped_label.to_reference}") let(:doc) { reference_filter("See #{scoped_label.to_reference}") }
it 'renders scoped label' do
expect(doc.css('.gl-label-scoped .gl-label-text').map(&:text)).to eq([scoped_label.scoped_label_key, scoped_label.scoped_label_value])
end
expect(doc.css('.gl-label-scoped .gl-label-text').map(&:text)).to eq([scoped_label.scoped_label_key, scoped_label.scoped_label_value]) it 'renders HTML tooltips' do
expect(doc.at_css('.gl-label-scoped a').attr('data-html')).to eq('true')
end
end end
it 'renders common label' do context 'with a common label' do
doc = reference_filter("See #{label.to_reference}") let(:doc) { reference_filter("See #{label.to_reference}") }
it 'renders common label' do
expect(doc.css('.gl-label .gl-label-text').map(&:text)).to eq([label.name])
end
expect(doc.css('.gl-label .gl-label-text').map(&:text)).to eq([label.name]) it 'renders non-HTML tooltips' do
expect(doc.at_css('.gl-label a').attr('data-html')).to be_nil
end
end end
end end
......
...@@ -119,3 +119,5 @@ module Banzai ...@@ -119,3 +119,5 @@ module Banzai
end end
end end
end end
Banzai::Filter::LabelReferenceFilter.prepend_if_ee('EE::Banzai::Filter::LabelReferenceFilter')
...@@ -55,7 +55,6 @@ module Banzai ...@@ -55,7 +55,6 @@ module Banzai
attributes[:reference_type] ||= self.class.reference_type attributes[:reference_type] ||= self.class.reference_type
attributes[:container] ||= 'body' attributes[:container] ||= 'body'
attributes[:placement] ||= 'top' attributes[:placement] ||= 'top'
attributes[:html] ||= 'true'
attributes.delete(:original) if context[:no_original_data] attributes.delete(:original) if context[:no_original_data]
attributes.map do |key, value| attributes.map do |key, value|
%Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}")
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Gitlab module Gitlab
module MarkdownCache module MarkdownCache
# Increment this number every time the renderer changes its output # Increment this number every time the renderer changes its output
CACHE_COMMONMARK_VERSION = 23 CACHE_COMMONMARK_VERSION = 24
CACHE_COMMONMARK_VERSION_START = 10 CACHE_COMMONMARK_VERSION_START = 10
BaseError = Class.new(StandardError) BaseError = Class.new(StandardError)
......
...@@ -75,6 +75,12 @@ RSpec.describe Banzai::Filter::IssueReferenceFilter do ...@@ -75,6 +75,12 @@ RSpec.describe Banzai::Filter::IssueReferenceFilter do
expect(doc.text).to eq "Issue #{reference}" expect(doc.text).to eq "Issue #{reference}"
end end
it 'renders non-HTML tooltips' do
doc = reference_filter("Issue #{reference}")
expect(doc.at_css('a')).not_to have_attribute('data-html')
end
it 'includes default classes' do it 'includes default classes' do
doc = reference_filter("Issue #{reference}") doc = reference_filter("Issue #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue has-tooltip' expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue has-tooltip'
......
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