From 023dd2907b4afa0bae5f8482cae75e1edd6954a8 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Fri, 29 May 2015 19:01:12 -0400
Subject: [PATCH] Add a `pipeline` context option for SanitizationFilter

When this option is `:description`, we use a more restrictive whitelist.
This is used for Project and Group description fields.
---
 app/views/groups/show.html.haml               |  2 +-
 app/views/projects/_home_panel.html.haml      |  2 +-
 lib/gitlab/markdown.rb                        |  3 +
 lib/gitlab/markdown/sanitization_filter.rb    | 64 ++++++++++++-------
 .../markdown/sanitization_filter_spec.rb      | 14 ++++
 5 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index f42007da073..0687840af39 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -11,7 +11,7 @@
       @#{@group.path}
     - if @group.description.present?
       .description
-        = markdown(@group.description)
+        = markdown(@group.description, pipeline: :description)
   %hr
 
   = render 'shared/show_aside'
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index 05f44acd3cb..076afb11a9d 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -5,7 +5,7 @@
   .project-home-row.project-home-row-top
     .project-home-desc
       - if @project.description.present?
-        = markdown(@project.description)
+        = markdown(@project.description, pipeline: :description)
       - if can?(current_user, :admin_project, @project)
         &ndash;
         = link_to 'Edit', edit_namespace_project_path
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 5db1566f55d..fa9c0975bb8 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -57,6 +57,9 @@ module Gitlab
       pipeline = HTML::Pipeline.new(filters)
 
       context = {
+        # SanitizationFilter
+        pipeline: options[:pipeline],
+
         # EmojiFilter
         asset_root: Gitlab.config.gitlab.url,
         asset_host: Gitlab::Application.config.asset_host,
diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb
index 88781fea0c8..fc29d09081a 100644
--- a/lib/gitlab/markdown/sanitization_filter.rb
+++ b/lib/gitlab/markdown/sanitization_filter.rb
@@ -8,33 +8,53 @@ module Gitlab
     # Extends HTML::Pipeline::SanitizationFilter with a custom whitelist.
     class SanitizationFilter < HTML::Pipeline::SanitizationFilter
       def whitelist
-        whitelist = super
-
-        # Only push these customizations once
-        unless customized?(whitelist[:transformers])
-          # Allow code highlighting
-          whitelist[:attributes]['pre'] = %w(class)
-          whitelist[:attributes]['span'] = %w(class)
-
-          # Allow table alignment
-          whitelist[:attributes]['th'] = %w(style)
-          whitelist[:attributes]['td'] = %w(style)
-
-          # Allow span elements
-          whitelist[:elements].push('span')
-
-          # Remove `rel` attribute from `a` elements
-          whitelist[:transformers].push(remove_rel)
-
-          # Remove `class` attribute from non-highlight spans
-          whitelist[:transformers].push(clean_spans)
+        # Descriptions are more heavily sanitized, allowing only a few elements.
+        # See http://git.io/vkuAN
+        if pipeline == :description
+          whitelist = LIMITED
+        else
+          whitelist = super
         end
 
+        customize_whitelist(whitelist)
+
         whitelist
       end
 
       private
 
+      def pipeline
+        context[:pipeline] || :default
+      end
+
+      def customized?(transformers)
+        transformers.last.source_location[0] == __FILE__
+      end
+
+      def customize_whitelist(whitelist)
+        # Only push these customizations once
+        return if customized?(whitelist[:transformers])
+
+        # Allow code highlighting
+        whitelist[:attributes]['pre'] = %w(class)
+        whitelist[:attributes]['span'] = %w(class)
+
+        # Allow table alignment
+        whitelist[:attributes]['th'] = %w(style)
+        whitelist[:attributes]['td'] = %w(style)
+
+        # Allow span elements
+        whitelist[:elements].push('span')
+
+        # Remove `rel` attribute from `a` elements
+        whitelist[:transformers].push(remove_rel)
+
+        # Remove `class` attribute from non-highlight spans
+        whitelist[:transformers].push(clean_spans)
+
+        whitelist
+      end
+
       def remove_rel
         lambda do |env|
           if env[:node_name] == 'a'
@@ -53,10 +73,6 @@ module Gitlab
           end
         end
       end
-
-      def customized?(transformers)
-        transformers.last.source_location[0] == __FILE__
-      end
     end
   end
 end
diff --git a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
index 4a1aa766149..80f3d2f2634 100644
--- a/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/sanitization_filter_spec.rb
@@ -42,6 +42,13 @@ module Gitlab::Markdown
     end
 
     describe 'custom whitelist' do
+      it 'customizes the whitelist only once' do
+        instance = described_class.new('Foo')
+        3.times { instance.whitelist }
+
+        expect(instance.whitelist[:transformers].size).to eq 4
+      end
+
       it 'allows syntax highlighting' do
         exp = act = %q{<pre class="code highlight white c"><code><span class="k">def</span></code></pre>}
         expect(filter(act).to_html).to eq exp
@@ -87,5 +94,12 @@ module Gitlab::Markdown
         expect(doc.at_css('a')['href']).to be_nil
       end
     end
+
+    context 'when pipeline is :description' do
+      it 'uses a stricter whitelist' do
+        doc = filter('<h1>My Project</h1>', pipeline: :description)
+        expect(doc.to_html.strip).to eq 'My Project'
+      end
+    end
   end
 end