Commit 59ecd2c9 authored by Douwe Maan's avatar Douwe Maan Committed by Rémy Coutable

Merge branch 'markdown-xss-fix-option-2.1' into 'security'

Fix for HackerOne XSS vulnerability in markdown

This is an updated blacklist patch to fix https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2007. No text is removed. Dangerous schemes/protocols and invalid URIs are left intact but not linked.

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23153

See merge request !2015
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent f77a64b4
---
title: Fix XSS issue in Markdown autolinker
merge_request:
author:
......@@ -71,6 +71,14 @@ module Banzai
@doc = parse_html(rinku)
end
# Return true if any of the UNSAFE_PROTOCOLS strings are included in the URI scheme
def contains_unsafe?(scheme)
return false unless scheme
scheme = scheme.strip.downcase
Banzai::Filter::SanitizationFilter::UNSAFE_PROTOCOLS.any? { |protocol| scheme.include?(protocol) }
end
# Autolinks any text matching LINK_PATTERN that Rinku didn't already
# replace
def text_parse
......@@ -89,8 +97,15 @@ module Banzai
doc
end
def autolink_filter(text)
text.gsub(LINK_PATTERN) do |match|
def autolink_match(match)
# start by stripping out dangerous links
begin
uri = Addressable::URI.parse(match)
return match if contains_unsafe?(uri.scheme)
rescue Addressable::URI::InvalidURIError
return match
end
# Remove any trailing HTML entities and store them for appending
# outside the link element. The entity must be marked HTML safe in
# order to be output literally rather than escaped.
......@@ -100,6 +115,9 @@ module Banzai
options = link_options.merge(href: match)
content_tag(:a, match, options) + dropped
end
def autolink_filter(text)
text.gsub(LINK_PATTERN) { |match| autolink_match(match) }
end
def link_options
......
......@@ -99,6 +99,28 @@ describe Banzai::Filter::AutolinkFilter, lib: true do
expect(doc.at_css('a')['href']).to eq link
end
it 'autolinks rdar' do
link = 'rdar://localhost.com/blah'
doc = filter("See #{link}")
expect(doc.at_css('a').text).to eq link
expect(doc.at_css('a')['href']).to eq link
end
it 'does not autolink javascript' do
link = 'javascript://alert(document.cookie);'
doc = filter("See #{link}")
expect(doc.at_css('a')).to be_nil
end
it 'does not autolink bad URLs' do
link = 'foo://23423:::asdf'
doc = filter("See #{link}")
expect(doc.to_s).to eq("See #{link}")
end
it 'does not include trailing punctuation' do
doc = filter("See #{link}.")
expect(doc.at_css('a').text).to eq link
......
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