Identity Server Security Bug: Bad HTML filtering regexp

[alexhiggins732]

Bad HTML filtering regexp

The original Identity Server 4 code base has several security bugs detected by CodeQL scanning violating this rule.

Description:

It is possible to match some single HTML tags using regular expressions (parsing general HTML using regular expressions is impossible). However, if the regular expression is not written well it might be possible to circumvent it, which can lead to cross-site scripting or other security issues.

Some of these mistakes are caused by browsers having very forgiving HTML parsers, and will often render invalid HTML containing syntax errors. Regular expressions that attempt to match HTML should also recognize tags containing such syntax errors.

Examples

Tool	Rule ID	Source
CodeQL	js/redos
/dist/jquery.js#L5960-L5960

	// checked="checked" or checked
	rchecked = /checked\s*(?:[^=]|=\s*.checked.)/i,
	rcleanScript = /^\s*<!(?:\[CDATA\[|--)|(?:\]\]|--)>\s*$/g;

Issues

• Issue 111 - https://github.com/alexhiggins732/IdentityServer8/security/code-scanning/111
• Issue 112 - https://github.com/alexhiggins732/IdentityServer8/security/code-scanning/112
• Issue 113 - https://github.com/alexhiggins732/IdentityServer8/security/code-scanning/113
• Issue 114 - https://github.com/alexhiggins732/IdentityServer8/security/code-scanning/114

Recommendation

Use a well-tested sanitization or parser library if at all possible. These libraries are much more likely to handle corner cases correctly than a custom implementation.

Example

The following example attempts to filters out all <script> tags.

function filterScript(html) {
var scriptRegex = /<script\b[^>]>([\s\S]?)</script>/gi;
var match;
while ((match = scriptRegex.exec(html)) !== null) {
html = html.replace(match[0], match[1]);
}
return html;
}

The above sanitizer does not filter out all <script> tags. Browsers will not only accept </script> as script end tags, but also tags such as </script foo="bar"> even though it is a parser error. This means that an attack string such as <script>alert(1)</script foo="bar"> will not be filtered by the function, and alert(1) will be executed by a browser if the string is rendered as HTML.

Other corner cases include that HTML comments can end with --!>, and that HTML tag names can contain upper case characters.

References

• Securitum: The Curious Case of Copy & Paste.
• stackoverflow.com: You can't parse [X]HTML with regex.
• HTML Standard: Comment end bang state.
• stackoverflow.com: Why aren't browsers strict about HTML?.
• Common Weakness Enumeration: CWE-20.
• Common Weakness Enumeration: CWE-80.
• Common Weakness Enumeration: CWE-116.
• Common Weakness Enumeration: CWE-184.
• Common Weakness Enumeration: CWE-185.
• Common Weakness Enumeration: CWE-186.

[alexhiggins732]

Security bugs have been patched.