Merge branch 'fix-mermaid-xss' into 'security-10-4'

[10.4] Fix stored XSS in code blocks

See merge request gitlab/gitlabhq!2316

10 files changed, 131 insertions, 16 deletions

diff --git a/app/assets/javascripts/render_mermaid.js b/app/assets/javascripts/render_mermaid.js
index 31c7a772cf4..d4f18955bd2 100644
--- a/app/assets/javascripts/render_mermaid.js
+++ b/app/assets/javascripts/render_mermaid.js
@@ -30,6 +30,9 @@ export default function renderMermaid($els) {
     $els.each((i, el) => {
       const source = el.textContent;
 
+      // Remove any extra spans added by the backend syntax highlighting.
+      Object.assign(el, { textContent: source });
+
       mermaid.init(undefined, el, (id) => {
         const svg = document.getElementById(id);
 
diff --git a/changelogs/unreleased/fix-stored-xss-in-code-blocks.yml b/changelogs/unreleased/fix-stored-xss-in-code-blocks.yml
new file mode 100644
index 00000000000..b595459ee6b
--- /dev/null
+++ b/changelogs/unreleased/fix-stored-xss-in-code-blocks.yml
@@ -0,0 +1,5 @@
+---
+title: Fix stored XSS in code blocks that ignore highlighting
+merge_request:
+author:
+type: security
diff --git a/db/schema.rb b/db/schema.rb
index b8a29d8c046..7250a7452c0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180113220114) do
+ActiveRecord::Schema.define(version: 20180122154930) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb
index a79a0154846..0ac7e231b5b 100644
--- a/lib/banzai/filter/syntax_highlight_filter.rb
+++ b/lib/banzai/filter/syntax_highlight_filter.rb
@@ -14,23 +14,33 @@ module Banzai
       end
 
       def highlight_node(node)
-        code = node.text
         css_classes = 'code highlight js-syntax-highlight'
-        language = node.attr('lang')
+        lang = node.attr('lang')
+        retried = false
 
-        if use_rouge?(language)
-          lexer = lexer_for(language)
+        if use_rouge?(lang)
+          lexer = lexer_for(lang)
           language = lexer.tag
+        else
+          lexer = Rouge::Lexers::PlainText.new
+          language = lang
+        end
+
+        begin
+          code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, node.text), tag: language)
+          css_classes << " #{language}" if language
+        rescue
+          # Gracefully handle syntax highlighter bugs/errors to ensure users can
+          # still access an issue/comment/etc. First, retry with the plain text
+          # filter. If that fails, then just skip this entirely, but that would
+          # be a pretty bad upstream bug.
+          return if retried
 
-          begin
-            code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, code), tag: language)
-            css_classes << " #{language}"
-          rescue
-            # Gracefully handle syntax highlighter bugs/errors to ensure
-            # users can still access an issue/comment/etc.
+          language = nil
+          lexer = Rouge::Lexers::PlainText.new
+          retried = true
 
-            language = nil
-          end
+          retry
         end
 
         highlighted = %(<pre class="#{css_classes}" lang="#{language}" v-pre="true"><code>#{code}</code></pre>)
diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb
index d8f1a919522..d8f1a919522 100644
--- a/spec/features/copy_as_gfm_spec.rb
+++ b/spec/features/markdown/copy_as_gfm_spec.rb
diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/markdown/gitlab_flavored_markdown_spec.rb
index 3c2186b3598..3c2186b3598 100644
--- a/spec/features/gitlab_flavored_markdown_spec.rb
+++ b/spec/features/markdown/gitlab_flavored_markdown_spec.rb
diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb
index a2b78a5e021..a2b78a5e021 100644
--- a/spec/features/markdown_spec.rb
+++ b/spec/features/markdown/markdown_spec.rb
diff --git a/spec/features/markdown/math_spec.rb b/spec/features/markdown/math_spec.rb
new file mode 100644
index 00000000000..6a23d6b78ab
--- /dev/null
+++ b/spec/features/markdown/math_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe 'Math rendering', :js do
+  it 'renders inline and display math correctly' do
+    description = <<~MATH
+      This math is inline $`a^2+b^2=c^2`$.
+
+      This is on a separate line
+      ```math
+      a^2+b^2=c^2
+      ```
+    MATH
+
+    project = create(:project, :public)
+    issue = create(:issue, project: project, description: description)
+
+    visit project_issue_path(project, issue)
+
+    expect(page).to have_selector('.katex .mord.mathit', text: 'b')
+    expect(page).to have_selector('.katex-display .mord.mathit', text: 'b')
+  end
+end
diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb
new file mode 100644
index 00000000000..a25d701ee35
--- /dev/null
+++ b/spec/features/markdown/mermaid_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe 'Mermaid rendering', :js do
+  it 'renders Mermaid diagrams correctly' do
+    description = <<~MERMAID
+      ```mermaid
+      graph TD;
+        A-->B;
+        A-->C;
+        B-->D;
+        C-->D;
+      ```
+    MERMAID
+
+    project = create(:project, :public)
+    issue = create(:issue, project: project, description: description)
+
+    visit project_issue_path(project, issue)
+
+    %w[A B C D].each do |label|
+      expect(page).to have_selector('svg foreignObject', text: label)
+    end
+  end
+end
diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
index 9f2efa05a01..ef52c572898 100644
--- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
+++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb
@@ -3,35 +3,86 @@ require 'spec_helper'
 describe Banzai::Filter::SyntaxHighlightFilter do
   include FilterSpecHelper
 
+  shared_examples "XSS prevention" do |lang|
+    it "escapes HTML tags" do
+      # This is how a script tag inside a code block is presented to this filter
+      # after Markdown rendering.
+      result = filter(%{<pre lang="#{lang}"><code>&lt;script&gt;alert(1)&lt;/script&gt;</code></pre>})
+
+      expect(result.to_html).not_to include("<script>alert(1)</script>")
+      expect(result.to_html).to include("alert(1)")
+    end
+  end
+
   context "when no language is specified" do
     it "highlights as plaintext" do
       result = filter('<pre><code>def fun end</code></pre>')
+
       expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">def fun end</span></code></pre>')
     end
+
+    include_examples "XSS prevention", ""
   end
 
   context "when a valid language is specified" do
     it "highlights as that language" do
       result = filter('<pre><code lang="ruby">def fun end</code></pre>')
+
       expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" lang="ruby" v-pre="true"><code><span id="LC1" class="line" lang="ruby"><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></span></code></pre>')
     end
+
+    include_examples "XSS prevention", "ruby"
   end
 
   context "when an invalid language is specified" do
     it "highlights as plaintext" do
       result = filter('<pre><code lang="gnuplot">This is a test</code></pre>')
+
       expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" lang="plaintext" v-pre="true"><code><span id="LC1" class="line" lang="plaintext">This is a test</span></code></pre>')
     end
+
+    include_examples "XSS prevention", "gnuplot"
   end
 
-  context "when Rouge formatting fails" do
+  context "languages that should be passed through" do
+    %w(math mermaid plantuml).each do |lang|
+      context "when #{lang} is specified" do
+        it "highlights as plaintext but with the correct language attribute and class" do
+          result = filter(%{<pre><code lang="#{lang}">This is a test</code></pre>})
+
+          expect(result.to_html).to eq(%{<pre class="code highlight js-syntax-highlight #{lang}" lang="#{lang}" v-pre="true"><code><span id="LC1" class="line" lang="#{lang}">This is a test</span></code></pre>})
+        end
+
+        include_examples "XSS prevention", lang
+      end
+    end
+  end
+
+  context "when Rouge lexing fails" do
     before do
-      allow_any_instance_of(Rouge::Formatter).to receive(:format).and_raise(StandardError)
+      allow_any_instance_of(Rouge::Lexers::Ruby).to receive(:stream_tokens).and_raise(StandardError)
     end
 
     it "highlights as plaintext" do
       result = filter('<pre><code lang="ruby">This is a test</code></pre>')
-      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight" lang="" v-pre="true"><code>This is a test</code></pre>')
+
+      expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight" lang="" v-pre="true"><code><span id="LC1" class="line" lang="">This is a test</span></code></pre>')
+    end
+
+    include_examples "XSS prevention", "ruby"
+  end
+
+  context "when Rouge lexing fails after a retry" do
+    before do
+      allow_any_instance_of(Rouge::Lexers::PlainText).to receive(:stream_tokens).and_raise(StandardError)
+    end
+
+    it "does not add highlighting classes" do
+      result = filter('<pre><code>This is a test</code></pre>')
+
+      expect(result.to_html).to eq('<pre><code>This is a test</code></pre>')
     end
+
+    include_examples "XSS prevention", "ruby"
   end
 end