Closed Bug 1528997 (CVE-2021-23974) Opened 6 years ago Closed 5 years ago

mXSS: Potential XSS via noscript tags parsed by DOMParser APIs

Categories

(Core :: DOM: Serializers, defect, P3)

67 Branch
defect

Tracking

()

RESOLVED FIXED
86 Branch
Tracking Status
firefox-esr78 --- fixed
firefox84 --- wontfix
firefox85 --- wontfix
firefox86 --- fixed

People

(Reporter: masatokinugawa, Assigned: hsivonen)

References

Details

(Keywords: csectype-other, reporter-external, sec-moderate, Whiteboard: [adv-main86+])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

Firefox decodes HTML entities inside <noscript> tags when it is parsed by DOMParser APIs.

For example:
A <noscript> B &lt;/noscript&gt; C &lt;img src=x onerror=alert(1)&gt; D </noscript> E

This HTML will be:

A <noscript> B </noscript> C <img src=x onerror=alert(1)> D </noscript> E

You can check this behavior: https://vulnerabledoma.in/fx_mxss_domparser_noscript.html

By abusing this behavior, an attacker might do XSS attacks by changing safe HTML to XSS-able HTML.
This type of bug is known as "mXSS" (mutation-based XSS). For more information, see this paper: https://cure53.de/fp170.pdf

I attached the same HTML file as the above PoC.

Actual results:

Firefox's DOMParser API decodes HTML entities inside <noscript> tags.

Expected results:

Firefox's DOMParser API should not decode HTML entities inside <noscript> tags.

Henri, can you take a look?

Group: firefox-core-security → dom-core-security
Component: Untriaged → HTML: Parser
Flags: needinfo?(hsivonen)
Product: Firefox → Core

Our behavior is compliant to the spec:
https://w3c.github.io/DOM-Parsing/#the-domparser-interface

It seems that Blink, WebKit and EdgeHTML don't comply with the spec. annevk, I guess the other engines have quorum and we should treat this as a spec bug?

Flags: needinfo?(hsivonen) → needinfo?(annevk)

Yeah, I suppose so. Note that this is also what https://xhr.spec.whatwg.org/#document-response says and what the HTML Standard claims in a note in the script element's processing model. It's not immediately obvious where we'd prevent these scripts from executing without this flag though I suppose this flag could only affect execution and not parsing of <noscript> or we turn it into a tri-state or some such...

Flags: needinfo?(annevk)

If we change our behavior we might want to note that on MDN when we eventually ship the change in the section on "changes for developers" (whatever it's called) that the release notes link to.

It seems that Blink, WebKit and EdgeHTML don't comply with the spec.

At least, HTML tags inside noscript tags are enabled on other browsers as well.
For example, this code returns the same result on Firefox, Chrome, Safari and Edge.

doc=new DOMParser().parseFromString('<noscript><p>test</p></noscript>','text/html');
doc.getElementsByTagName('p')[0]+""//[object HTMLParagraphElement]

But why does Firefox return decoded HTML entities via the innerHTML? This issue is here. I'm curious if this behavior is written in the spec.

Priority: -- → P2

Masato, thank you for that clarification. In https://html.spec.whatwg.org/#html-fragment-serialisation-algorithm it says for serializing Text nodes:

If the parent of current node is a style, script, xmp, iframe, noembed, noframes, or plaintext element, or if the parent of current node is a noscript element and scripting is enabled for the node, then append the value of current node's data IDL attribute literally.

Otherwise, append the value of current node's data IDL attribute, escaped as described below.

And scripting is enabled is defined as:

Scripting is enabled for a node if the node's node document has a browsing context, and scripting is enabled in that browsing context.

so that might well be a bug in Firefox's HTML serializer.

(I was thinking about the serializer when I saw this bug, apologies for not investigating more fully.)

https://searchfox.org/mozilla-central/source/dom/base/nsHTMLContentSerializer.cpp does not have this logic (and also seems to lack xmp and other things present in the standard)...

Status: UNCONFIRMED → NEW
Component: HTML: Parser → Serializers
Ever confirmed: true

Clearing priority etc. so this gets retriaged based on comment #7. Pinging dveditz to be sure.

Flags: needinfo?(dveditz)
Priority: P2 → --

(In reply to Anne (:annevk) from comment #7)

(I was thinking about the serializer when I saw this bug, apologies for not investigating more fully.)

https://searchfox.org/mozilla-central/source/dom/base/nsHTMLContentSerializer.cpp does not have this logic (and also seems to lack xmp and other things present in the standard)...

FWIW, that's the Save As... serializer. The innerHTML serializer is nsContentUtils::SerializeNodeToMarkup.

Thanks, per https://searchfox.org/mozilla-central/source/dom/base/nsContentUtils.cpp#8982 it doesn't have this noscript branching either, but there's a comment there about the standard being wrong. After a bunch of tracing it seems the noscript comment traces back to bug 744830. Though perhaps it's also present in the code that was based on?

Flags: needinfo?(bugs)

I edited Masato's original comment to fix the code formatting for clarity. Hope that's OK!

Hard to rate as a security bug. Not a direct threat to Firefox, but could contribute to XSS on some site. Not sure how many sites put user content in <noscript> tags these days and would then invoke the parser on it, but if they did it'd be bad for users of that site. Is this scenario likely to happen on a site with sensitive user data? who knows. sec-moderate?

Flags: needinfo?(dveditz)
Priority: -- → P3
See Also: → 1683627

Because we use this sanitizer in our own exploit migitation we have decided to award it a bounty. This was fixed in bug 1683627

Depends on: 1683627
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty+

Thanks, Masato!

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

(To clarify, this should be fixed by the other bug. I can't reproduce it on Firefox Nightly)

Assignee: nobody → hsivonen
Group: dom-core-security → core-security-release
Target Milestone: --- → 86 Branch
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Whiteboard: [adv-main86+]
Attached file advisory.txt
Alias: CVE-2021-23974
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: