diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/helpers/gitlab_markdown_helper.rb | 1 | ||||
| -rw-r--r-- | doc/markdown/markdown.md | 2 | ||||
| -rw-r--r-- | lib/gitlab/markdown.rb | 34 | ||||
| -rw-r--r-- | spec/helpers/gitlab_markdown_helper_spec.rb | 30 |
5 files changed, 58 insertions, 10 deletions
diff --git a/CHANGELOG b/CHANGELOG index d4a1346c481..7c3e5dfcb31 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.10.0 (unreleased) + - Allow HTML tags in Markdown input - Include missing events and fix save functionality in admin service template settings form (Stan Hu) - Fix "Import projects from" button to show the correct instructions (Stan Hu) - Fix dots in Wiki slugs causing errors (Stan Hu) diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 08221aaa2f8..7ca3f058636 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -35,7 +35,6 @@ module GitlabMarkdownHelper user_color_scheme_class, { # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch- - filter_html: true, with_toc_data: true, safe_links_only: true }.merge(options)) diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md index 181db287201..965d8fc313f 100644 --- a/doc/markdown/markdown.md +++ b/doc/markdown/markdown.md @@ -441,6 +441,8 @@ Note that inline HTML is disabled in the default Gitlab configuration, although <dd>Does *not* work **very** well. Use HTML <em>tags</em>.</dd> </dl> +See the documentation for HTML::Pipeline's [SanitizationFilter](http://www.rubydoc.info/gems/html-pipeline/HTML/Pipeline/SanitizationFilter#WHITELIST-constant) class for the list of allowed HTML tags and attributes. In addition to the default `SanitizationFilter` whitelist, GitLab allows the `class`, `id`, and `style` attributes. + ## Horizontal Rule ``` diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index f5e8267031c..41bb8d08924 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -79,15 +79,35 @@ module Gitlab # Used markdown pipelines in GitLab: # GitlabEmojiFilter - performs emoji replacement. + # SanitizationFilter - remove unsafe HTML tags and attributes # # see https://gitlab.com/gitlab-org/html-pipeline-gitlab for more filters filters = [ - HTML::Pipeline::Gitlab::GitlabEmojiFilter + HTML::Pipeline::Gitlab::GitlabEmojiFilter, + HTML::Pipeline::SanitizationFilter ] + whitelist = HTML::Pipeline::SanitizationFilter::WHITELIST + whitelist[:attributes][:all].push('class', 'id') + whitelist[:elements].push('span') + + # Remove the rel attribute that the sanitize gem adds, and remove the + # href attribute if it contains inline javascript + fix_anchors = lambda do |env| + name, node = env[:node_name], env[:node] + if name == 'a' + node.remove_attribute('rel') + if node['href'] && node['href'].match('javascript:') + node.remove_attribute('href') + end + end + end + whitelist[:transformers].push(fix_anchors) + markdown_context = { asset_root: Gitlab.config.gitlab.url, - asset_host: Gitlab::Application.config.asset_host + asset_host: Gitlab::Application.config.asset_host, + whitelist: whitelist } markdown_pipeline = HTML::Pipeline::Gitlab.new(filters).pipeline @@ -97,18 +117,14 @@ module Gitlab if options[:xhtml] saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML end - text = result[:output].to_html(save_with: saveoptions) - allowed_attributes = ActionView::Base.sanitized_allowed_attributes - allowed_tags = ActionView::Base.sanitized_allowed_tags + text = result[:output].to_html(save_with: saveoptions) - text = sanitize text.html_safe, - attributes: allowed_attributes + %w(id class style), - tags: allowed_tags + %w(table tr td th) if options[:parse_tasks] text = parse_tasks(text) end - text + + text.html_safe end private diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index ddbb4467f10..c631acc591d 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -727,6 +727,36 @@ describe GitlabMarkdownHelper do expected = "" expect(markdown(actual)).to match(expected) end + + it 'should allow whitelisted HTML tags from the user' do + actual = '<dl><dt>Term</dt><dd>Definition</dd></dl>' + expect(markdown(actual)).to match(actual) + end + + it 'should sanitize tags that are not whitelisted' do + actual = '<textarea>no inputs allowed</textarea> <blink>no blinks</blink>' + expected = 'no inputs allowed no blinks' + expect(markdown(actual)).to match(expected) + expect(markdown(actual)).not_to match('<.textarea>') + expect(markdown(actual)).not_to match('<.blink>') + end + + it 'should allow whitelisted tag attributes from the user' do + actual = '<a class="custom">link text</a>' + expect(markdown(actual)).to match(actual) + end + + it 'should sanitize tag attributes that are not whitelisted' do + actual = '<a href="http://example.com/bar.html" foo="bar">link text</a>' + expected = '<a href="http://example.com/bar.html">link text</a>' + expect(markdown(actual)).to match(expected) + end + + it 'should sanitize javascript in attributes' do + actual = %q(<a href="javascript:alert('foo')">link text</a>) + expected = '<a>link text</a>' + expect(markdown(actual)).to match(expected) + end end describe 'markdown for empty repository' do |