Defect #44634
open
CommonMark no longer escapes the tags covered by the tagfilter extension removed in commonmarker 2.10.1
Description
After updating the commonmarker gem from 2.10.0 to 2.10.1, the following test fails:
Failure:
Redmine::WikiFormatting::CommonMark::FormatterTest#test_should_escape_unwanted_tags [test/unit/lib/redmine/wiki_formatting/common_mark/formatter_test.rb:278]:
--- expected
+++ actual
@@ -1 +1 @@
-"<p>sit<br>amet <style>.foo { color: #fff; }</style> <script>alert(\"hello world\");</script></p>"
+"<p>sit<br>amet .foo { color: #fff; } </p>"
bin/rails test test/unit/lib/redmine/wiki_formatting/common_mark/formatter_test.rb:271
With commonmarker 2.10.0, the <style> and <script> tags in the input are escaped and shown as text. With 2.10.1, they are removed from the output instead.
Files
Updated by Go MAEDA about 4 hours ago
- Target version set to 7.1.0
Pinned the commonmarker gem to 2.10.0 in r25267 as a temporary workaround for this issue.
Updated by Go MAEDA 10 minutes ago
- File 0001-Escape-tags-that-commonmarker-escaped-with-its-tagfi.patch 0001-Escape-tags-that-commonmarker-escaped-with-its-tagfi.patch added
- Subject changed from CommonMark formatter test fails with commonmarker 2.10.1 to CommonMark no longer escapes the tags covered by the tagfilter extension removed in commonmarker 2.10.1
The cause of this issue is that commonmarker 2.10.1 removed the tagfilter extension. This follows the removal of the extension in comrak 0.56.0, the Rust library that commonmarker is built on. The author of comrak removed it as neither sufficient nor necessary for safety (see comrak#829 and comrak#845). Although the release notes of commonmarker 2.10.1 do not mention this change, the description of the tagfilter option has been removed from its README.md.
commonmarker silently ignores unknown options, so the tagfilter: true setting in Redmine::WikiFormatting::CommonMark::PIPELINE_CONFIG no longer has any effect. As a result, tags such as <script> and <style> in raw HTML are no longer escaped. Instead, they are removed by the sanitizer, which causes the test failure.
The attached patch escapes the same nine tags that the tagfilter extension escaped (title, textarea, style, xmp, iframe, noembed, noframes, script, and plaintext; see Disallowed Raw HTML in the GFM specification). It does so in the HTML output of commonmarker, before the output is parsed and sanitized. The patch also removes the tagfilter option, which is now ignored.
As far as security is concerned, the sanitizer is sufficient, since it removes these tags even without the tagfilter extension. However, relying on the sanitizer alone breaks the display of existing content. The sanitizer works on the parsed document, and when parsing an unclosed tag of these kinds, the HTML parser treats the rest of the text as its content. For example, with commonmarker 2.10.1 and without the patch, the following text:
Unclosed <script> tag Next paragraph
is rendered as:
<p>Unclosed </p>
Everything after the <script> tag, including the following paragraph, is lost. With an unclosed <title> or <iframe> tag, the rest of the text is shown as HTML source instead. Existing content that was displayed correctly with commonmarker 2.10.0 would therefore be broken after upgrading.
The patch adds a test for the unclosed tag case. All tests in test/unit/lib/redmine/wiki_formatting/common_mark/ and test/helpers/application_helper_test.rb pass with both commonmarker 2.10.0 and 2.10.1.