Commit 685dd0c5 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-150-xss-reference-redactor' into 'master'

Fix XSS in Banzai's `#data_attributes_for`

Closes #150

See merge request gitlab-org/security/gitlab!576
parents e8b935e5 aa0e9b33
---
title: Fix stored XSS in markdown renderer
merge_request:
author:
type: security
......@@ -30,7 +30,7 @@ module EE
def data_attributes_for(text, group, object, link_content: false, link_reference: false)
{
original: text,
original: escape_html_entities(text),
link: link_content,
link_reference: link_reference,
group: group.id,
......
......@@ -62,7 +62,7 @@ RSpec.describe Banzai::Filter::EpicReferenceFilter do
link = doc.css('a').first
expect(link).to have_attribute('data-original')
expect(link.attr('data-original')).to eq(reference)
expect(link.attr('data-original')).to eq(CGI.escapeHTML(reference))
end
it 'ignores invalid epic IIDs' do
......
......@@ -253,7 +253,7 @@ module Banzai
object_parent_type = parent.is_a?(Group) ? :group : :project
{
original: text,
original: escape_html_entities(text),
link: link_content,
link_reference: link_reference,
object_parent_type => parent.id,
......
......@@ -3,7 +3,7 @@
module Gitlab
module MarkdownCache
# Increment this number every time the renderer changes its output
CACHE_COMMONMARK_VERSION = 22
CACHE_COMMONMARK_VERSION = 23
CACHE_COMMONMARK_VERSION_START = 10
BaseError = Class.new(StandardError)
......
......@@ -20,6 +20,18 @@ RSpec.describe Banzai::Filter::AbstractReferenceFilter do
end
end
describe '#data_attributes_for' do
let_it_be(:issue) { create(:issue, project: project) }
it 'is not an XSS vector' do
allow(described_class).to receive(:object_class).and_return(Issue)
data_attributes = filter.data_attributes_for('xss <img onerror=alert(1) src=x>', project, issue, link_content: true)
expect(data_attributes[:original]).to eq('xss <img onerror=alert(1) src=x>')
end
end
describe '#parent_per_reference' do
it 'returns a Hash containing projects grouped per parent paths' do
expect(filter).to receive(:references_per_parent)
......
......@@ -24,7 +24,7 @@ RSpec.describe Banzai::Pipeline::FullPipeline do
it 'escapes the data-original attribute on a reference' do
markdown = %Q{[">bad things](#{issue.to_reference})}
result = described_class.to_html(markdown, project: project)
expect(result).to include(%{data-original='\">bad things'})
expect(result).to include(%{data-original='\">bad things'})
end
end
......
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