summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2018-01-10 12:27:56 +0000
committerSean McGivern <sean@gitlab.com>2018-02-01 18:07:28 +0000
commitdd7416a604a80e78fc1c08edc9e7b96ebea3059b (patch)
tree7a62ce9f1ef1d5d0b8013b5b5a33eac573754d3f
parentfb829ac5f90eb3ebc8f9e63cd1bffb201d82ed39 (diff)
downloadgitlab-ce-dd7416a604a80e78fc1c08edc9e7b96ebea3059b.tar.gz
Fix stored XSS in code blocks
There were three things here: 1. Display math was broken. 2. <script> tags could be injected into code blocks with the language as `math`, `mermaid`, or `plantuml`. 3. <script> tags could be injected if Rouge threw an exception, for whatever reason. This fixes all of those by always using the same code path for 'standard' highlighting and 'special' languages (mathematics, Mermaid, and PlantUML), and skipping the filter entirely if Rouge fails on a retry with the plain text filter. It also adds specs for KaTeX and Mermaid rendering.
-rw-r--r--changelogs/unreleased/fix-stored-xss-in-code-blocks.yml5
-rw-r--r--lib/banzai/filter/syntax_highlight_filter.rb34
-rw-r--r--spec/lib/banzai/filter/syntax_highlight_filter_spec.rb57
3 files changed, 81 insertions, 15 deletions
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/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/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