Summary
The DisallowedRawHtml extension does not escape a disallowed tag when the tag name is the last thing in the raw HTML. A Markdown line containing just <script is emitted unchanged, and the next block can supply its attributes. With the shipped GFM defaults this allows stored XSS by anyone who can post Markdown.
Details
DisallowedRawHtmlRenderer escapes tags with this regex:
/<(\/?(?:title|textarea|style|xmp|iframe|noembed|noframes|script|plaintext)[\s\/>])/i
The trailing character class requires one character after the tag name. The block parser does not: RegexHelper::PARTIAL_HTMLBLOCKOPEN accepts end of line after a tag name, so <script alone opens an HTML block. Because a rendered HtmlBlock has no trailing newline, the regex has nothing to match and the < passes through.
In the browser the newline is still present, so the tag name terminates there and whatever follows becomes attributes.
This is the same filter that GHSA-4v6x-c7xx-hw9f fixed in 2.8.1. That fix widened the character class but still requires one character, so this case was not covered.
Reproduction
Render this with GithubFlavoredMarkdownConverter and default settings:
<div>
<script
<span src="/evil.js">
Output:
<div>
<script
<span src="/evil.js">
A browser parses that as <script src="/evil.js"> with a junk <span attribute, and the script runs. <iframe with <span onload="..."> works the same way and does not need a later </script> in the page.
Control: <script src="/evil.js"></script> is correctly escaped to <script src="/evil.js"></script>.
Affected versions
1.3.0 (when the extension was added) through the current release. The </style and mid-line forms are only affected as continuation lines inside an already-open HTML block.
Preconditions
html_input is allow (the default)- The
DisallowedRawHtml extension is active, which the GFM extension enables automatically
- Untrusted users can post Markdown
Setting html_input to escape or strip fully mitigates this.
Suggested fix
Allow end of string after the tag name:
$regex = \sprintf('/<(\/?(?:%s))([\s\/>]|$)/i', \implode('|', \array_map('preg_quote', $tags)));
return \preg_replace($regex, '<$1$2', $rendered);This escapes every bypass shape above and leaves <div>, <scripts> and <span class="a"> untouched. The existing unit test only covers tag names followed by another character, so a case for a bare tag name should be added.
References
4n86rakam1 Reporter
Summary
The
DisallowedRawHtmlextension does not escape a disallowed tag when the tag name is the last thing in the raw HTML. A Markdown line containing just<scriptis emitted unchanged, and the next block can supply its attributes. With the shipped GFM defaults this allows stored XSS by anyone who can post Markdown.Details
DisallowedRawHtmlRendererescapes tags with this regex:The trailing character class requires one character after the tag name. The block parser does not:
RegexHelper::PARTIAL_HTMLBLOCKOPENaccepts end of line after a tag name, so<scriptalone opens an HTML block. Because a renderedHtmlBlockhas no trailing newline, the regex has nothing to match and the<passes through.In the browser the newline is still present, so the tag name terminates there and whatever follows becomes attributes.
This is the same filter that GHSA-4v6x-c7xx-hw9f fixed in 2.8.1. That fix widened the character class but still requires one character, so this case was not covered.
Reproduction
Render this with
GithubFlavoredMarkdownConverterand default settings:Output:
A browser parses that as
<script src="/evil.js">with a junk<spanattribute, and the script runs.<iframewith<span onload="...">works the same way and does not need a later</script>in the page.Control:
<script src="/evil.js"></script>is correctly escaped to<script src="/evil.js"></script>.Affected versions
1.3.0 (when the extension was added) through the current release. The
</styleand mid-line forms are only affected as continuation lines inside an already-open HTML block.Preconditions
html_inputisallow(the default)DisallowedRawHtmlextension is active, which the GFM extension enables automaticallySetting
html_inputtoescapeorstripfully mitigates this.Suggested fix
Allow end of string after the tag name:
This escapes every bypass shape above and leaves
<div>,<scripts>and<span class="a">untouched. The existing unit test only covers tag names followed by another character, so a case for a bare tag name should be added.References