Skip to content

Escape "<" and ">" in attributes when serializing HTML #6235

Description

@securityMB

I'm submitting this issue after a short discussion on Twitter with @zcorpan today.

I think we should change the rules of escaping a string in attribute mode, and also escape < and > to &lt; and &gt; respectively.

The fact that these characters are not escaped led to some security issues in HTML parsers and sanitizers.

As an example, see this DOMPurify bypass. The bug was that the following markup

<svg></p>

was parsed into the following DOM tree in Chromium and Safari:

┗ svg svg
  ┗ html p

Now because the typical usage of sanitizers is as follows:

elem.innerHTML = Sanitizer.sanitize(markup)

it means that the markup is serialized and then reparsed.

Now consider the following markup:

<svg></p><style><a title="</style><img src onerror=alert(1)>">

which is parsed into the following DOM tree:

┗ svg svg
  ┣ html p
  ┗ svg style
    ┗ svg a title="</style><img src onerror=alert(1)">

It doesn't contain any harmful markup, so it is serialized to:

<svg><p></p><style><a title="</style><img src onerror=alert(1)>"></style></svg>

However, after reparsing a different DOM tree is created:

┣ svg svg
┣ html p
┣ html style
┃ ┗ #text: <a title="
┣ html img src="" onerror="alert(1)"
┗ #text: ">

Leading to cross-site scripting. The reason for that is in fact that p breaks out foreign content.

Please note that if < and > were escaped, then the markup would be serialized to:

<svg><p></p><style><a title="&lt;/style&gt;&lt;img src onerror=alert(1)&gt;"></style></svg>

Making this particular bypass (and many similar ones) impossible.

Activity

  1. domenic commented on Dec 17, 2020

    @domenic
    Member

    cc @whatwg/html-parser

  2. added
    security-trackerGroup bringing to attention of security, or tracked by the security Group but not needing response.
    security/privacyThere are security or privacy implications
    on Dec 18, 2020
  3. abhijitbavdhankar commented on Dec 26, 2020

    @abhijitbavdhankar

    Example like, DOMPurify bypass is website/email old virus when you click on advertisement image/button/hyperlink/text.
    It was resolved by checking malicious code during HTML DOM Parsing.

    SOLUTION

    < and > to < and >
    Change at code level as per your requirement like *lt".

  4. SelenIT commented on Jan 5, 2021

    @SelenIT

    IMO, the true reason of the described behavior is that <style> is a raw text element so there is no "elements" and "attributes" inside it from the HTML DOM perspective, only text content (as correctly shown in the 3rd DOM example, with the #text: <a title=" child node of the style element). Changing this behavior doesn't seem to be web compatible since at least the > character is widely used in in-page styles as a CSS child combinator.

    Could you please explain why do you expect <svg></p><style><a title="</style><img src onerror=alert(1)>"> and <svg><p></p><style><a title="</style><img src onerror=alert(1)>"></style></svg> to be parsed differently?

  5. annevk commented on Jan 5, 2021

    @annevk
    Member

    @SelenIT because there it's an SVG style element and those parse differently. Foreign content is tricky.

  6. SelenIT commented on Jan 5, 2021

    @SelenIT

    I understand the difference between HTML style and SVG style. I didn't get why the same HTML p element breaks the foreign element parsing in the second case and doesn't break in the first. Is it the difference introduced by the "fragment case" (https://html.spec.whatwg.org/#parsing-main-inforeign)?

  7. securityMB commented on Jan 5, 2021

    @securityMB
    Author

    @SelenIT

    Parsing of <svg></p> is actually a spec bug that still works in Safari. Check: #5113

    [edit]: But the spec bug isn't the only case in which escaping of < and > in attributes would help. Check this famous XSS in Google Search video.

    It had the following payload:

    <noscript><p title="</noscript><img src onerror=alert(1)>">

    It abused the difference in NOSCRIPT parsing when scripting is enabled and disabled. If the code would be serialized to:

    <noscript><p title="&lt;/noscript&gt;&lt;img src onerror=alert(1)&gt;"></p></noscript>

    then the XSS would also not have been possible.

  8. SelenIT commented on Jan 5, 2021

    @SelenIT

    Thanks a lot for the explanation!

    I agree that escaping these characters will prevent the XSS in these cases. But isn't the existence of parsing modes/cases where attribute-like sequences aren't actually parsed as attributes the significant part of the problem here?

  9. zcorpan commented on Jan 25, 2021

    @zcorpan
    Member

    @SelenIT are there known mutation XSS exploits with parsing modes that don't also use "<" or ">" in an attribute value?

  10. zcorpan commented on Jan 25, 2021

    @zcorpan
    Member

    I think this change is a good idea and is probably doable.

    The main unknown is the web compat risk. In 2008, all browsers except IE escaped "<" and ">" in attribute values, and the spec was changed based on feedback from myself, citing a single page that was broken in Opera but worked in IE. I don't recall any fallout of newly broken pages from making that change back then. However, it's been over a decade, and new content might have come to rely on the current behavior.

    Any breakage here seems hard to find through static analysis, since it requires innerHTML or something to serialize some HTML, and then something else to expect unescaped "<"s or ">"s. The example from 2008 was something like this:

    <a href="javascript: if (foo < 10) doSomething()" id="theLink">link</a>
    ...
    <script>
    theLink.onclick = function() {
      eval(this.outerHTML.match(/href="([^"]+)"/)[1]);
      return false;
    }
    </script>
    

    Yes, extremely silly code, but it's stuff like this that can break.

    Unless someone comes up with an idea of how to identify and measure regressions beforehand, I think I would suggest experimenting with making this change in a browser and see what if anything breaks during the dev/beta period.

  11. 40 remaining items

  12. securityMB commented on Jun 4, 2025

    @securityMB
    Author

    @zcorpan This means that it will be available on the stable channel for everyone in Firefox 140, is that correct? (I'm currently preparing a blog post about this change so this information will be useful there!)

  13. zcorpan commented on Jun 4, 2025

    @zcorpan
    Member

    Yes.

  14. ADKaster commented on Jun 13, 2025

    @ADKaster
    Contributor

    fwiw @securityMB if your blog post is still in progress, the fix is in ladybird as of a few weeks ago as well. LadybirdBrowser/ladybird#4839.

  15. securityMB commented on Jun 13, 2025

    @securityMB
    Author
  16. securityMB commented on Jun 23, 2025

    @securityMB
    Author

    Looks like we have the first bug report caused by this change: https://issues.chromium.org/issues/425325063 (Figma affected). This should be easy to fix though.

  17. mozfreddyb commented on Jun 23, 2025

    @mozfreddyb
    Contributor

    Is there a fix you envision other than asking the website to make it work on their end?

  18. securityMB commented on Jun 23, 2025

    @securityMB
    Author

    I think it's one of these cases where the website itself has to be fixed. The only way to "fix" it on our end would be to revert the change.

  19. jcubic commented on Aug 2, 2025

    @jcubic
  20. securityMB commented on Aug 3, 2025

    @securityMB
    Author
  21. MasterInQuestion commented on Aug 31, 2025

    @MasterInQuestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    normative changesecurity-trackerGroup bringing to attention of security, or tracked by the security Group but not needing response.security/privacyThere are security or privacy implicationstopic: parser

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions