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

[10.4] Fix stored XSS in code blocks

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);
 

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

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)
+        if use_rouge?(lang)
-          lexer = lexer_for(language)
+          lexer = lexer_for(lang)
           language = lexer.tag
+        else
+          lexer = Rouge::Lexers::PlainText.new
+          language = lang
+        end
 
         begin
-            code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, code), tag: language)
+          code = Rouge::Formatters::HTMLGitlab.format(lex(lexer, node.text), tag: language)
-            css_classes << " #{language}"
+          css_classes << " #{language}" if language
         rescue
-            # Gracefully handle syntax highlighter bugs/errors to ensure
+          # Gracefully handle syntax highlighter bugs/errors to ensure users can
-            # users can still access an issue/comment/etc.
+          # 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
 
           language = nil
-          end
+          lexer = Rouge::Lexers::PlainText.new
+          retried = true
+
+          retry
         end
 
         highlighted = %(<pre class="#{css_classes}" lang="#{language}" v-pre="true"><code>#{code}</code></pre>)

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

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

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 "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 formatting fails" do
+  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>')
+
-    end
+      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