Commit b5b475c2 authored by Cindy Pallares's avatar Cindy Pallares

Merge branch 'security-xss-in-markdown-following-unrecognized-html-element' into 'master'

[master] XSS in markdown following unrecognized HTML element

Closes #2732

See merge request gitlab/gitlabhq!2599
parent c4bb0a11
...@@ -15,7 +15,7 @@ module CacheMarkdownField ...@@ -15,7 +15,7 @@ module CacheMarkdownField
# Increment this number every time the renderer changes its output # Increment this number every time the renderer changes its output
CACHE_REDCARPET_VERSION = 3 CACHE_REDCARPET_VERSION = 3
CACHE_COMMONMARK_VERSION_START = 10 CACHE_COMMONMARK_VERSION_START = 10
CACHE_COMMONMARK_VERSION = 11 CACHE_COMMONMARK_VERSION = 12
# changes to these attributes cause the cache to be invalidates # changes to these attributes cause the cache to be invalidates
INVALIDATED_BY = %w[author project].freeze INVALIDATED_BY = %w[author project].freeze
......
---
title: Fix possible XSS attack in Markdown urls with spaces
merge_request: 2599
author:
type: security
...@@ -17,6 +17,9 @@ module Banzai ...@@ -17,6 +17,9 @@ module Banzai
# This is a small extension to the CommonMark spec. If they start allowing # This is a small extension to the CommonMark spec. If they start allowing
# spaces in urls, we could then remove this filter. # spaces in urls, we could then remove this filter.
# #
# Note: Filter::SanitizationFilter should always be run sometime after this filter
# to prevent XSS attacks
#
class SpacedLinkFilter < HTML::Pipeline::Filter class SpacedLinkFilter < HTML::Pipeline::Filter
include ActionView::Helpers::TagHelper include ActionView::Helpers::TagHelper
......
...@@ -12,13 +12,16 @@ module Banzai ...@@ -12,13 +12,16 @@ module Banzai
def self.filters def self.filters
@filters ||= FilterArray[ @filters ||= FilterArray[
Filter::PlantumlFilter, Filter::PlantumlFilter,
# Must always be before the SanitizationFilter to prevent XSS attacks
Filter::SpacedLinkFilter,
Filter::SanitizationFilter, Filter::SanitizationFilter,
Filter::SyntaxHighlightFilter, Filter::SyntaxHighlightFilter,
Filter::MathFilter, Filter::MathFilter,
Filter::ColorFilter, Filter::ColorFilter,
Filter::MermaidFilter, Filter::MermaidFilter,
Filter::SpacedLinkFilter,
Filter::VideoLinkFilter, Filter::VideoLinkFilter,
Filter::ImageLazyLoadFilter, Filter::ImageLazyLoadFilter,
Filter::ImageLinkFilter, Filter::ImageLinkFilter,
......
...@@ -104,5 +104,17 @@ describe Banzai::Pipeline::GfmPipeline do ...@@ -104,5 +104,17 @@ describe Banzai::Pipeline::GfmPipeline do
expect(output).to include("src=\"test%20image.png\"") expect(output).to include("src=\"test%20image.png\"")
end end
it 'sanitizes the fixed link' do
markdown_xss = "[xss](javascript: alert%28document.domain%29)"
output = described_class.to_html(markdown_xss, project: project)
expect(output).not_to include("javascript")
markdown_xss = "<invalidtag>\n[xss](javascript:alert%28document.domain%29)"
output = described_class.to_html(markdown_xss, project: project)
expect(output).not_to include("javascript")
end
end end
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