alexhiggins732 / IdentityServer8

DotNet 8, Identity, OpenID Connect and OAuth 2.0 Framework for ASP.NET Core Identity Server 8
Apache License 2.0
53 stars 25 forks source link

Identity Server Security Bug: Bad HTML filtering regexp #22

Open alexhiggins732 opened 8 months ago

alexhiggins732 commented 8 months ago

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

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

alexhiggins732 commented 8 months ago

Security bugs have been patched.