1667113 - (CVE-2020-26951) mXSS payload (combined svg, style, title element) lead to HTML Sanitizer Bypass and XSS via Clipboard API (paste)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Irvan Kurniawan (:sourc7)]

Attachment: uxss-svg-style-title.html

When continuing fuzzing the Firefox Nightly, I found another HTML sanitizer bypass with payload <svg><style><title>[xss payload].
The impact leads to HTML sanitizer bypass on privileged page and UXSS via clipboard API (paste).

Although DOM tree on document.body.innerHTML show <svg><style></style></svg>, but script execution is still possible.

Several XSS payloads I tested and it works:

• <svg><style><title><audio src/onerror=alert(14)>
• <svg><style><title><img src/onerror=alert(14)>
• <svg><style><title><input src type=image onerror=alert(14)>

(and another [xss payload] probably will works..)

Browser tested:

• Firefox Nightly 83.0a1 (2020-09-23) (64-bit)
• Firefox Release 81.0 (64-bit)
• Firefox ESR 78.3.0esr (64-bit)

Steps to reproduce:

HTML sanitizer bypass on privileged page

1. Go to about:config
2. Set security.csp.enable boolean from true -> false
3. Restart Firefox Nightly
4. Go to about:config
5. Open Dev Tools (Ctrl+Shift+I)
6. Click Console tab
7. Paste the code document.body.innerHTML = "<svg><style><title><audio src/onerror=alert(Components.utils.loadedComponents[0])>" to the browser console
8. XSS will execute

UXSS via clipboard API (paste)

1. Download attached file "uxss-svg-style-title.html" (Sorry can't share direct-link, because GCP cloud console outage in Asia)
2. Click Copy Text
3. Go to https://output.jsbin.com/gocizet/ (TinyMCE Rich Editor form)
4. Paste with Ctrl+V or Right click -> Paste into TinyMCE form
5. XSS will execute [pop alert(document.domain)]

Comment 1

[Daniel Veditz [:dveditz]]

Freddy: there are two bugs here. In the second bug do you know if TinyMCE is using DOMPurify, and maybe a version susceptible to the recently fixed mXSS bugs?

I'm not sure the 1st one is a bug. if you're turning off our defenses it's not a "bypass", and innerHTML doesn't sanitize anything. What does the reporter think is doing the sanitization here?

Comment 2

[:Gijs (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #1)

I'm not sure the 1st one is a bug. if you're turning off our defenses it's not a "bypass", and innerHTML doesn't sanitize anything. What does the reporter think is doing the sanitization here?

This is based on https://blog.mozilla.org/security/2019/12/02/help-test-firefoxs-built-in-html-sanitizer-to-protect-against-uxss-bugs/ - the sanitization is done in the HTML tree sanitizer when assigning to innerHTML - so it should sanitize things. The reason to turn off CSP is because even when the first line of defense (the HTML sanitization) fails, in about:config there's another one (namely the CSP). We don't use CSP everywhere (for... reasons), so it's worthwhile to find out when the sanitization fails.

Comment 3

[Irvan Kurniawan (:sourc7)]

(In reply to Daniel Veditz [:dveditz] from comment #1)

Freddy: there are two bugs here. In the second bug do you know if TinyMCE is using DOMPurify, and maybe a version susceptible to the recently fixed mXSS bugs?

I'm not sure the 1st one is a bug. if you're turning off our defenses it's not a "bypass", and innerHTML doesn't sanitize anything. What does the reporter think is doing the sanitization here?

On the 1st bug I demonstrate bypassing the HTML sanitizer with something that could execute JS by directly testing the HTML sanitizer

In the 1st scenario at step 2 I only disable CSP to demonstrate the payload able to execute the script. (If the CSP not disabled, CSP will block script execution with message "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)).

Next on CSP disabled condition, usually after inserting document.body.innerHTML = "<audio src/onerror=alert(Components.utils.loadedComponents[0])>" I still received following message (from HTML sanitizer):

Removed unsafe attribute. Element: audio. Attribute: onerror. debugger eval code:1
Removed unsafe URI from element attribute. Element: audio. Attribute: src.

It shows Firefox sanitizes the HTML markup, preventing script execution.

When I insert document.body.innerHTML = "<svg><style><title><audio src/onerror=alert(Components.utils.loadedComponents[0])>" to the browser console, the result I didn't receive warning message from the HTML sanitizer, and onerror script is also executed.

On 2st bug I forgot to mention it not only affect TinyMCE, it also affect other rich editor (with contenteditable & not override browser paste with preventdefault). For demonstration I've created contenteditable HTML on http://output.jsbin.com/kulewar.

Comment 4

[Frederik Braun [:freddy]]

It seems(In reply to Daniel Veditz [:dveditz] from comment #1)

Freddy: there are two bugs here. In the second bug do you know if TinyMCE is using DOMPurify, and maybe a version susceptible to the recently fixed mXSS bugs?

No, this is not related to code in web content.
We generally need to run a sanitizer upon Copy & Paste. All browsers do, but it's very poorly specified. Indeed, one could demo this on contenteditable elements instead of TinyMCE and other editors.
For reference see non-normative section on "Pasting HTML" in the Editor's Draft for Clipboard API or our code at https://searchfox.org/mozilla-central/rev/670e13b51d272125c76a1bf84b9f3707583cde12/editor/libeditor/HTMLEditorDataTransfer.cpp#3543

Comment 5

[Henri Sivonen (:hsivonen)]

Without testing, my guess is that we remove element children of SVG style but since disconnected image nodes load their src for compat with Netscape <= 4.x, we end up loading processing the loading for the empty-string src and running the onerror handler.

Perhaps we need to change node removal in the sanitizer to iterate over the subtree rooted at the node being removed and remove all attributes in it.

Comment 6

[Henri Sivonen (:hsivonen)]

I expect this to be a duplicate of bug 1666300.

Comment 7

[Daniel Veditz [:dveditz]]

Agree with Henri this sounds like bug 1666300 (and kmag's synthesis in bug 1666893). Since this is "worse" because he shows the sanitizer is affected, and is requesting a bounty, I don't want to call it an outright duplicate. I'll leave it "depends on" for now, but probably for bounty purposes there's only one bug here (and we'll take the worst form of it).

Comment 8

[Daniel Veditz [:dveditz]]

How is this different from your bug 1666777 though, which also shows that the underlying 1666300 problem bypasses the sanitizer?

Comment 9

[Irvan Kurniawan (:sourc7)]

(In reply to Daniel Veditz [:dveditz] from comment #7)

Agree with Henri this sounds like bug 1666300 (and kmag's synthesis in bug 1666893). Since this is "worse" because he shows the sanitizer is affected, and is requesting a bounty, I don't want to call it an outright duplicate. I'll leave it "depends on" for now, but probably for bounty purposes there's only one bug here (and we'll take the worst form of it).

Hi Daniel, thank you for set the severity to high.
This is as my expectation 😃 (which refer to Client Bug Bounty Program)

Hi (In reply to Daniel Veditz [:dveditz] from comment #8)

How is this different from your bug 1666777 though, which also shows that the underlying 1666300 problem bypasses the sanitizer?

Sorry if this was duplicate to bug 1666300, because on bug 166300, I discovered XSS payload <svg><style><image href=1 onerror=alert(document.domain)> execute uXSS on paste and able to bypass the sanitizer (refer to ticket 1666777).

While I'm keep fuzzing on the same root HTML element <svg><style>, I'm unable to find another XSS payload pair (e.g. <audio>, <img>, <input>) to execute the onevent handler, only <image href=1 onerror=alert(document.domain)> works that able to execute. I was think it probably caused by Firefox implementation on <image href> on SVG as Frederik also said on ticket 166300.

On the next day, while I'm fuzzing HTML element <svg><style><title>[xss payload], surprisingly it execute the HTML element (on [xss payload] e.g. <audio>, <img>, <input>) onevent handler. Also only <title> element tree works, when I tried replace <title> with another HTML element (e.g. <head>, <body>, and more..) it won't execute.

After that when changing HTML element tree on [xss payload] to <svg><style><title><image href=1 onerror=alert(document.domain)> as payload on bug 166300, the onevent handler won't execute.

From my perspective the payload <svg><style><title>[xss payload] is different from bug 166300 <svg><style><image href=1 onerror=alert(document.domain), it's reason why I created new ticket (bug 1667113).

Although payload is different, but both bug 166300 and bug 1667113 is relying on same root HTML element <svg><style>. Which make me believe when Mozilla team fix bug on ticket 166300, this bug will fixed too.

Comment 10

[Kagami Rosylight [:saschanaz] (they/them)]

How is it different from <svg><foreignObject><img src/onerror=alert(14)> which does not show this bypassing issue? What prevents onerror from running while it does not in the <svg><style> case?

Comment 11

[Kris Maglione [:kmag]]

(In reply to Kagami :saschanaz from comment #10)

How is it different from <svg><foreignObject><img src/onerror=alert(14)> which does not show this bypassing issue? What prevents onerror from running while it does not in the <svg><style> case?

In the former case, the event handler attributes are actually stripped from the element.

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

Ah I see, so in the <style> case the sanitizer removes the whole <img> while in <foreignObject> case it only removes the attribute. Comment #5 seems to make sense to prevent this.

Comment 13

[Henri Sivonen (:hsivonen)]

The patch currently attached to bug 1666300 fixes the XSS part of this (i.e. the event handler part) but the fetch is still attempted but gets blocked by CSP. If we wanted to prevent the fetch attempt that now gets blocked by CSP, we'd probably need to deoptimize the chrome innerHTML setter to parse into a throw-away "loaded as data" document.

Unclear how we should proceed given that it would be defense in depth with a perf cost.

Comment 14

[Kris Maglione [:kmag]]

I think we can live with the fetch being attempted for now. As I've said elsewhere, the flags we use for automatic chrome sanitization doesn't remove images, and the sanitizer won't remove http:/https: URIs, anyway, which I think are the most likely ones to be able to cause side-effects.

That said, I think fragment parsing in chrome documents should probably be an uncommon enough thing at this point that we can probably afford parsing into a data document as long as we don't create a new one each time. Node adoption should be relatively cheap, especially if we haven't created JS reflectors for the nodes.

Comment 15

[Henri Sivonen (:hsivonen)]

(In reply to Kris Maglione [:kmag] from comment #14)

I think we can live with the fetch being attempted for now. As I've said elsewhere, the flags we use for automatic chrome sanitization doesn't remove images, and the sanitizer won't remove http:/https: URIs, anyway, which I think are the most likely ones to be able to cause side-effects.

That said, I think fragment parsing in chrome documents should probably be an uncommon enough thing at this point that we can probably afford parsing into a data document as long as we don't create a new one each time. Node adoption should be relatively cheap, especially if we haven't created JS reflectors for the nodes.

In the DOM Core meeting, we leaned towards using a throw-away doc despite the cost of moving the nodes to another doc upon insertion.

Comment 16

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1667113.

Comment 17

[Henri Sivonen (:hsivonen)]

Per the steps to reproduce, flipping security.csp.enable requires a restart. What's the proper way to write a test case for the sanitizer behavior in the chrome innerHTML case?

Comment 18

[Frederik Braun [:freddy]]

You don't need to disable the CSP to test this.
Instead of having the script actually fire, you can observe the csp-on-violate-policy topic like we do in https://searchfox.org/mozilla-central/rev/9c72508fcf2bba709a5b5b9eae9da35e0c707baa/dom/security/test/csp/test_link_rel_preload.html#21

Comment 19

[Henri Sivonen (:hsivonen)]

We can't make the throw-away doc too inert in the XML case, because we refuse to create XUL elements with a null principal.

Comment 20

[Henri Sivonen (:hsivonen)]

Moreover:

SecurityError: Element.innerHTML setter: Adopting nodes across docgroups in chrome documents is unsupported

Comment 21

[Henri Sivonen (:hsivonen)]

It appears that this comes from bug 1467970, which I don't have access to.

Comment 22

[:Gijs (he/him)]

(In reply to Henri Sivonen (:hsivonen) from comment #21)

It appears that this comes from bug 1467970, which I don't have access to.

I CC'd you.

Comment 23

[Henri Sivonen (:hsivonen)]

Thanks. In some ways, it seems questionable to have to make the throw-away docs here less obviously inert in order to please another security mechanism. Oh well.

Comment 24

[Henri Sivonen (:hsivonen)]

Hmm. My earlier code reading suggested that reusing the same script global would put the two docs in the same doc group, but apparently not!

Comment 25

[Henri Sivonen (:hsivonen)]

Sadly, we'll need to rely on just "loaded as data" without further attempts to make the throw-away doc have a null principal, a bogus URL, or no script handling object. Attempts to make the document abnormal like that trips up our previous security mechanisms.

Comment 26

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1667113 test - Test innerHTML sanitizer in chrome context.

Comment 27

[Henri Sivonen (:hsivonen)]

Comment on attachment 9181521 [details]
Bug 1667113.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Not easily, but this should land together with bug 1666300 whose part 1 make it pretty obvious what the problem is.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Either the same patch applies or it will be a mechanical rebase.
• How likely is this patch to cause regressions; how much testing does it need?: Not very likely: Regressions are likely to break the entire mochitest test harness (and did during development), so now that the harness works again, I expect things to be OK.

Comment 29

[Tom Ritter [:tjr] (OOTO until mid-August)]

(In reply to Irvan Kurniawan from comment #9)

(In reply to Daniel Veditz [:dveditz] from comment #7)

Agree with Henri this sounds like bug 1666300 (and kmag's synthesis in bug 1666893). Since this is "worse" because he shows the sanitizer is affected, and is requesting a bounty, I don't want to call it an outright duplicate. I'll leave it "depends on" for now, but probably for bounty purposes there's only one bug here (and we'll take the worst form of it).

Hi Daniel, thank you for set the severity to high.
This is as my expectation 😃 (which refer to Client Bug Bounty Program)

Just a note that for the purposes of the Exploit Mitigation Bounty; the bug may not be marked as sec-high; but the mitigation bounty would still apply as the bug would be considered 'High Impact' per the table on the page. If we make a mistake about the payout amount and don't treat it as High Impact, feel free to raise the concern with [e-mail]

Comment 30

[Tom Ritter [:tjr] (OOTO until mid-August)]

Comment on attachment 9181521 [details]
Bug 1667113.

Approved to land and uplift. Test approved to land after 12/15

Comment 31

[Julien Cristau [:jcristau]]

Landed:
https://hg.mozilla.org/integration/autoland/rev/29c5257d0a06bf200ceb41d12d1674c4d3087168

Backed out for perma failures on browser_parsable_css.js:
https://hg.mozilla.org/integration/autoland/rev/104bf955a8130a41673f410b7c139edcd85ec0a8

Comment 32

[Henri Sivonen (:hsivonen)]

Is it certain that the backed-out changesets are to blame? I see browser_parsable_css.js fail in directory iteration, and a Pernosco trace indicates that it doesn't even get far enough to run the code added here.

Comment 33

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The backfills of the failing task indicated a permanent failure which started with this push and is gone after its backout. (Ignore the 2 bc3 tasks between the one at the top and the last failed one - they don't contain the failing test.)

Comment 34

[Henri Sivonen (:hsivonen)]

Hmm. Before getting to what it actually wants to test, the test first iterates over lots of files and directories in JS, which is very slow in a debug build and even slower under rr. This takes such a long time, that it probably just timed out for me. I'll try a new Pernosco run with the timeout increased.

Comment 35

[:Gijs (he/him)]

FWIW, in case it's helpful, https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=S_VZPZjoQM2arxf3psr85g.0&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&searchStr=os%2Cx%2C10.14%2Copt%2Cmochitests%2Ctest-macosx1014-64%2Fopt-mochitest-browser-chrome-e10s%2Cbc3&revision=e09fe2a6602b0c8d0b9ceb5f869456f01cefa445 shows that the failure is that line 511 in checkAllTheCSS fails. That line is https://hg.mozilla.org/integration/autoland/file/e09fe2a6602b0c8d0b9ceb5f869456f01cefa445/browser/base/content/test/static/browser_parsable_css.js#l511 :

  doc.head.innerHTML = "";

which threw an NS_ERROR_FAILURE exception (which is confusing; using an empty string to clear out an element shouldn't even really be invoking any sanitizer code, I wouldn't have thought?).

Edit: Oh, and this is the content document of an iframe which we remove from the DOM on the previous line.

It also failed on Linux and Windows and on opt builds, so it should be straightforward to reproduce locally on a non-debug build? I wouldn't be surprised if it also reproduced if you reduced the number of files it loads.

Comment 36

[Henri Sivonen (:hsivonen)]

Yeah, the iframe removal sets the script global object to nullptr. Since per review we error out if that has happened, we error out.

Comment 37

[Henri Sivonen (:hsivonen)]

Tweaked the test after checking with smaug.

Comment 38

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/integration/autoland/rev/9143f95d5ab3896f5461fa1666500f6fc1f9e73b

Comment 39

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9143f95d5ab3

Comment 40

[Irvan Kurniawan (:sourc7)]

Thanks Mozilla Team for the patch, I confirmed the issue has been resolved on Firefox Nightly 84.0a1 (2020-10-23) (64-bit).

But when directly testing the HTML sanitizer I notice that I no longer received any warning message from Firefox sanitizer (e.g. “Removed unsafe attribute. Element: img. Attribute: onerror.”) or CSP error message (e.g. "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)).

Comment 41

[Irvan Kurniawan (:sourc7)]

Also if this (In reply to Tom Ritter [:tjr] (ni? for response to sec-[advisories/bounties/ratings/cves]) from comment #29)

(In reply to Irvan Kurniawan from comment #9)

(In reply to Daniel Veditz [:dveditz] from comment #7)

Agree with Henri this sounds like bug 1666300 (and kmag's synthesis in bug 1666893). Since this is "worse" because he shows the sanitizer is affected, and is requesting a bounty, I don't want to call it an outright duplicate. I'll leave it "depends on" for now, but probably for bounty purposes there's only one bug here (and we'll take the worst form of it).

Hi Daniel, thank you for set the severity to high.
This is as my expectation 😃 (which refer to Client Bug Bounty Program)

Just a note that for the purposes of the Exploit Mitigation Bounty; the bug may not be marked as sec-high; but the mitigation bounty would still apply as the bug would be considered 'High Impact' per the table on the page. If we make a mistake about the payout amount and don't treat it as High Impact, feel free to raise the concern with [e-mail]

Thanks Tom for clarifying Exploit Mitigation Bounty to me, also if this eligible for HoF, I would like to credited as "Irvan Kurniawan (sourc7)".

Comment 42

[Irvan Kurniawan (:sourc7)]

(In reply to Irvan Kurniawan (:sourc7) from comment #40)

Thanks Mozilla Team for the patch, I confirmed the issue has been resolved on Firefox Nightly 84.0a1 (2020-10-23) (64-bit).

But when directly testing the HTML sanitizer I notice that I no longer received any warning message from Firefox sanitizer (e.g. “Removed unsafe attribute. Element: img. Attribute: onerror.”) or CSP error message (e.g. "Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”)).

Edit: I'm still able to receive CSP error message when submit another payload.

Comment 43

[Frederik Braun [:freddy]]

Edit: I'm still able to receive CSP error message when submit another payload.

Can you clarify? Do you say you found another issue with our sanitizer?

Comment 44

[Irvan Kurniawan (:sourc7)]

(In reply to Frederik Braun [:freddy] from comment #43)

Edit: I'm still able to receive CSP error message when submit another payload.

Can you clarify? Do you say you found another issue with our sanitizer?

No there no issue with CSP error message, it's my fault by submit payload document.body.innerHTML = "<img src=x>" which doesn't trigger CSP error message. Regarding to the sanitizer, I haven't found another sanitizer bypass.

I only notice I no longer received any warning message from Firefox sanitizer (e.g. “Removed unsafe attribute. Element: img. Attribute: onerror.”).

Comment 45

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f5cb05d63da5

Comment 47

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 48

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/1f27bcfc588f

Comment 49

[Frederik Braun [:freddy]]

Attachment: advisory.txt

Comment 50

[Tom Ritter [:tjr] (OOTO until mid-August)]

Attachment: advisory.txt

Comment 51

[Tom Ritter [:tjr] (OOTO until mid-August)]

Attachment: advisory.txt

Comment 52

[Henri Sivonen (:hsivonen)]

https://hg.mozilla.org/mozilla-central/rev/027514ba89006dfd5c346c307e46ed8f79e358c1