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/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, code), tag: language) - css_classes << " #{language}" - rescue - # Gracefully handle syntax highlighter bugs/errors to ensure - # users can still access an issue/comment/etc. + 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 - language = nil - end + language = nil + lexer = Rouge::Lexers::PlainText.new + retried = true + + retry end highlighted = %(
#{code}
)
diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/markdown/copy_as_gfm_spec.rb
similarity index 100%
rename from spec/features/copy_as_gfm_spec.rb
rename to 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
similarity index 100%
rename from spec/features/gitlab_flavored_markdown_spec.rb
rename to spec/features/markdown/gitlab_flavored_markdown_spec.rb
diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown/markdown_spec.rb
similarity index 100%
rename from spec/features/markdown_spec.rb
rename to 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(%{<script>alert(1)</script>
})
+
+ expect(result.to_html).not_to include("")
+ expect(result.to_html).to include("alert(1)")
+ end
+ end
+
context "when no language is specified" do
it "highlights as plaintext" do
result = filter('def fun end
')
+
expect(result.to_html).to eq('def fun end
')
end
+
+ include_examples "XSS prevention", ""
end
context "when a valid language is specified" do
it "highlights as that language" do
result = filter('def fun end
')
+
expect(result.to_html).to eq('def fun end
')
end
+
+ include_examples "XSS prevention", "ruby"
end
context "when an invalid language is specified" do
it "highlights as plaintext" do
result = filter('This is a test
')
+
expect(result.to_html).to eq('This is a test
')
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(%{This is a test
})
+
+ expect(result.to_html).to eq(%{This is a test
})
+ 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('This is a test
')
- expect(result.to_html).to eq('This is a test
')
+
+ expect(result.to_html).to eq('This is a test
')
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('This is a test
')
+
+ expect(result.to_html).to eq('This is a test
')
+ end
+
+ include_examples "XSS prevention", "ruby"
end
end