Change HTML sanitization

Use the `SanitizationFilter` class from the html-pipeline gem for inline
HTML instead of calling the Rails `sanitize` method.

3 files changed, 25 insertions, 72 deletions

diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb
index 7bafbbd5f3f..6df506e835d 100644
--- a/app/helpers/gitlab_markdown_helper.rb
+++ b/app/helpers/gitlab_markdown_helper.rb
@@ -49,7 +49,7 @@ module GitlabMarkdownHelper
                       space_after_headers: true,
                       superscript: true)
     end
-    @markdown.render(sanitize_html(text)).html_safe
+    @markdown.render(text).html_safe
   end
 
   # Return the first line of +text+, up to +max_chars+, after parsing the line
diff --git a/doc/markdown/markdown.md b/doc/markdown/markdown.md
index ddf1bbc6ee4..4ab73df8af9 100644
--- a/doc/markdown/markdown.md
+++ b/doc/markdown/markdown.md
@@ -440,64 +440,7 @@ 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>
 
-The following tags can be used:
-
-* `<a/>`
-* `<abbr/>`
-* `<acronym/>`
-* `<address/>`
-* `<b/>`
-* `<big/>`
-* `<blockquote/>`
-* `<br/>`
-* `<cite/>`
-* `<code/>`
-* `<dd/>`
-* `<del/>`
-* `<dfn/>`
-* `<div/>`
-* `<dl/>`
-* `<dt/>`
-* `<em/>`
-* `<h1/>`
-* `<h2/>`
-* `<h3/>`
-* `<h4/>`
-* `<h5/>`
-* `<h6/>`
-* `<hr/>`
-* `<i/>`
-* `<img/>`
-* `<ins/>`
-* `<kbd/>`
-* `<li/>`
-* `<ol/>`
-* `<p/>`
-* `<pre/>`
-* `<samp/>`
-* `<small/>`
-* `<span/>`
-* `<strong/>`
-* `<sub/>`
-* `<sup/>`
-* `<tt/>`
-* `<ul/>`
-* `<var/>`
-
-You can also use the following HTML attributes in your inline tags:
-
-* `abbr`
-* `alt`
-* `cite`
-* `class`
-* `datetime`
-* `height`
-* `href`
-* `name`
-* `src`
-* `title`
-* `width`
-* `xml:lang`
+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 32f04c866e3..cd70fd5e85b 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -79,15 +79,34 @@ 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', 'style')
+
+      # 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,22 +116,13 @@ module Gitlab
       if options[:xhtml]
         saveoptions |= Nokogiri::XML::Node::SaveOptions::AS_XHTML
       end
-      text = result[:output].to_html(save_with: saveoptions)
 
-      sanitize_html(text)
-    end
-
-    # Remove HTML tags and attributes that are not whitelisted
-    def sanitize_html(text)
-      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
     end