1909241 - (CVE-2024-7524) CSP strict-dynamic bypass via shimmed Facebook SDK + DOM Clobbering

Status: VERIFIED FIXED

(Web Compatibility :: Tooling & Investigations, defect)

Description

[Masato Kinugawa]

Attachment: fx_csp_bypass_strict-dynamic_fb.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36

Steps to reproduce:

Summary:
When Firefox's Strict Enhanced Tracking Protection is enabled, certain tracking contents are shimmed. Here, Facebook JS SDK shimmed by Firefox can be used as script gadgets to bypass Content Security Policy strict-dynamic. It allows an attacker to execute arbitrary JavaScript even if the page has strict CSP rules.

Note: In order to use this bypass, the page vulnerable to XSS must have the Facebook SDK loaded and additonally the existing legitimate code has to call FB.login() API. Otherwise, the vulnerable code will not be reached.

Steps to reproduce:

1. Open https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html with Strict Enhanced Tracking Protection enabled. This page contains strict CSP rules:

<meta http-equiv="Content-Security-Policy" content="default-src 'none';script-src 'strict-dynamic' 'nonce-random'">

2. Click on the "Login with Facebook" button to call FB.login(). An alert dialog will pop up.

Root cause:
This is caused by DOM clobbering resulting in a reference to the wrong element.
The DOM clobbering occurs in the document.currentScript.src access at: https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/facebook-sdk.js#29
Here, <img name="currentScript" src="data:,alert(document.domain)"> is accessed wrongly.
And then if the FB.login() is called, the wrong URL will be loaded via a <script> tag at https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/facebook-sdk.js#314

Note that the same references exist in other shims, which can be also potentially used for the bypass:
https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/rambler-authenticator.js#8
https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/vidible.js#18

The following seems to be test code, but just in case:
https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/live-test-shim.js#10
https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/mochitest-shim-1.js#10
https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/mochitest-shim-2.js#10

Actual results:

CSP strict-dynamic is bypassed.

Expected results:

CSP strict-dynamic should not be bypassed.

Comment 1

[Masato Kinugawa]

FYI, in Nightly build, the bypass also works via an injected class="fb-video".

Steps to reproduce (Nightly only):

1. Open https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb2.html with Strict Enhanced Tracking Protection enabled.
2. Click on the "Click to allow blocked Facebook content" area. An alert dialog will pop up.

The reason why this only works on Nightly is because of this isNightly check: https://searchfox.org/mozilla-central/rev/5756c5a3dea4f2896cdb3c8bb15d0ced5e2bf690/browser/extensions/webcompat/shims/facebook-sdk.js#161

Comment 2

[Masato Kinugawa]

Attachment: fx_csp_bypass_strict-dynamic_fb2.html (Nightly only)

I attached the Comment 1 PoC's copy.

Comment 3

[Emma Zühlcke [:emz]]

Tom since you're the author of that shim, could you please take a look?

Comment 4

[Thomas Wisniewski [:twisniewski]]

I have a patch ready which avoids reading directly from document.currentScript, but instead gets the URL from the content script which manages messaging for the shims. As far as I can see, in local testing this patch resolves both of the XSS mentioned above.

For reference, the three shims where this is a risk are fixed in the patch (FB, Rambler, and Vidible). From what I can it ought to be fine to keep using currentScript as-is in the other places where it's used by SmartBlock, as there is no XSS risk (several test files, and the Neilsen shim, which does no script injection).

Comment 5

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1909241 - shim updates; r=robwu

Comment 6

[Emma Zühlcke [:emz]]

Thanks for taking the bug Tom!

A question, maybe also for the security folks: How can we avoid this in the future. Is this something we could do static analysis on or somehow cover in automated tests?

Comment 7

[Rob Wu [:robwu]]

Patch is ready for sec-approval process.

Note: although this bug is rated sec-high, I think that it is actually sec-moderate, as its impact is similar to bug 1432358 which was also rated sec-moderate.

The impact of this bug is limited to websites that already have a HTML injection/XSS vulnerability. Web authors may use CSP with the expectation that they'd mitigate the impact of XSS. Due to the reported bug, the CSP "strict-dynamic" directive can be bypassed. Websites that do not use strict-dynamic, but e.g. an allowlist of permitted hosts are still safe.

EDIT: I chatted with Dan - this is borderline sec-high instead of sec-moderate because CSP strict-dynamic feature is more widely deployed now.

Comment 8

[Thomas Wisniewski [:twisniewski]]

Comment on attachment 9414742 [details]
Bug 1909241 - shim updates; r=robwu

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Someone with knowledge on XSS and DOM clobbering may notice that we're changing how we validate URLs in this patch, which would tip them off that DOM clobbering is possible. In that case I would imagine it would be easy to figure out.
• 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 branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: All supported branches, flags have not been set yet
• 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?: The patches should apply cleanly, save for the version number in the manifest not matching the one in the branches. For those, simply bumping the version number on that branch will suffice to reconcile the difference. I am also willing to make backport patches.
• How likely is this patch to cause regressions; how much testing does it need?: Low risk, testing needs to be manually done for now, and there are two links on the bug to proof of concepts with which to test. The user must visit the links in strict ETP (restarting the browser between each test), click on the targets in the test, and confirm that no alert box appears, after closing any login popups which may appear.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 9

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1909241 - shim updates;

Comment 10

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Security fix for an XSS when using strict ETP on sites with Facebook logins
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Enabled strict ETP, visit https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html , click the login button, close the Facebook login popup, and confirm that no other alert box has appeared saying "vulnerabledoma.in"
• Risk associated with taking this patch: Low
• Explanation of risk level: It only affects a SmartBlock feature to help login or view videos that are otherwise blocked by strict ETP or private browsing.
• String changes made/needed: none
• Is Android affected?: yes

Comment 11

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Security fix for CSP bypass when using strict ETP
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Enabled strict ETP, visit https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html , click the login button, close the Facebook login popup, and confirm that no other alert box has appeared saying "vulnerabledoma.in"
• Risk associated with taking this patch: Low
• Explanation of risk level: It only affects a SmartBlock feature to help login or view videos that are otherwise blocked by strict ETP or private browsing.
• String changes made/needed: none
• Is Android affected?: yes

Comment 12

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1909241 - shim updates;

Comment 13

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Security fix for CSP bypass when using strict ETP
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Enable strict ETP, visit https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html , click the login button, close the Facebook login popup, and confirm that no other alert box has appeared saying "vulnerabledoma.in"
• Risk associated with taking this patch: Low
• Explanation of risk level: It only affects a SmartBlock feature to help login or view videos that are otherwise blocked by strict ETP or private browsing.
• String changes made/needed: none
• Is Android affected?: yes

Comment 14

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1909241 - shim updates;

Comment 15

[Phabricator Automation]

esr115 Uplift Approval Request

• User impact if declined: Security fix for CSP bypass when using strict ETP
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: nabled strict ETP, visit https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html , click the login button, close the Facebook login popup, and confirm that no other alert box has appeared saying "vulnerabledoma.in"
• Risk associated with taking this patch: Low
• Explanation of risk level: It only affects a SmartBlock feature to help login or view videos that are otherwise blocked by strict ETP or private browsing.
• String changes made/needed: none
• Is Android affected?: yes

Comment 16

[Phabricator Automation]

esr115 Uplift Approval Request

• User impact if declined: Security fix for CSP bypass when using strict ETP
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: nabled strict ETP, visit https://vulnerabledoma.in/fx_csp_bypass_strict-dynamic_fb.html , click the login button, close the Facebook login popup, and confirm that no other alert box has appeared saying "vulnerabledoma.in"
• Risk associated with taking this patch: Low
• Explanation of risk level: It only affects a SmartBlock feature to help login or view videos that are otherwise blocked by strict ETP or private browsing.
• String changes made/needed: none
• Is Android affected?: yes

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 9414742 [details]
Bug 1909241 - shim updates; r=robwu

sec-approval+ = dveditz
please land right away, and uplift the beta version. Check with RelMan on ESR uplift timing/requests

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3fa91a2dd1c
shim updates; r=robwu

Comment 19

[Masato Kinugawa]

I checked the patch. I think the following part should be pathname, not path:

const { hostname, path, href } = new URL(src);

Even if this typo is fixed, there's still another problem. I believe if <img name="currentScript" src="data://connect.facebook.net/sdk.js?,alert(document.domain)"> is inserted, it bypasses strict-dynamic.
The following are the properties of new URL('data://connect.facebook.net/sdk.js?,alert(document.domain)')

hash: ""
host: "connect.facebook.net"​
hostname: "connect.facebook.net"​
href: "data://connect.facebook.net/sdk.js?,alert(document.domain)"​
origin: "null"​
password: ""​
pathname: "/sdk.js"​
port: ""​
protocol: "data:"​
search: "?,alert(document.domain)"​
username: ""

As can be seen, the hostname is connect.facebook.net and the pathname is /sdk.js, which bypasses the check.
could you please check this?

Comment 20

[Thomas Wisniewski [:twisniewski]]

Agreed, I missed those cases, thanks for the extra eyes. I'll post a follow-up patch, and once it's en-route, I will update my uplifts patches accordingly before we land those.

Comment 21

[Daniel Veditz [:dveditz]]

That's got to be a bug in our URL parser, right? Chrome doesn't do that (host and hostname are empty), and it's hard to imagine the spec gets it that wrong.

Comment 22

[Daniel Veditz [:dveditz]]

The URL spec ignores data: urls entirely, and the algorithm there says to do what we're doing. It's maybe a decent guess for unknown schemes (follows old URL syntax RFCs) but is clearly wrong for many common known schemes. This is was brought up years ago in https://github.com/whatwg/url/issues/385, and clearly Chrome has gone their own (correct) way here.

Comment 23

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1909241 - shim updates; r=dveditz

Comment 24

[Daniel Veditz [:dveditz]]

I did take a look to see if there are other places that might cause us similar problems. If someone clobbered a function like document.getElementById or document.createElement our code would throw, because an HTMLElement is not a function. It would likely break the web page itself so we can worry about that later. If we want to be totally safe all of our extensions will have to cache our own copy of the the "real" properties we want to use

Ignoring the function calls, then, the places we use document values look like these:
https://searchfox.org/mozilla-central/search?q=document%28%5C.%5Ba-zA-Z%5D%2B%5Cb%29%2B%5B%5E%28.%5D&path=extensions%2Fwebcompat%2F&case=false&regexp=true

It shouldn't matter if document.location.href is clobbered and spoofed in the rich-relevance.js shim as that's a value the web page would have gotten from the real script we're shimming so we haven't made it any worse.

There are a couple of potentially spoofed size values, and a clobbered document.readyState will always be !== "loading"

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d55ae483ed48
shim updates; r=dveditz

Comment 26

[Daniel Veditz [:dveditz]]

Re: comment 7:

I agree with you that a CSP bypass that requires an XSS bug on the victim site has been historically capped at sec-moderate. Bug 1432358 was arguably more severe because the resource was available to every site and not limited to only those who use the specific shimmed libraries. On the other hand, there's a lot more use of strict-dynamic now than there was 6 years ago, especially on popular sites. It's easier to manage, and there have been some papers showing the host whitelist approach often leads to unexpected holes.

It was close enough to a sec-high that mccr8 and I were both worried this wouldn't get prioritized as a sec-moderate.

Comment 27

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

https://hg.mozilla.org/mozilla-central/rev/d3fa91a2dd1c
https://hg.mozilla.org/mozilla-central/rev/d55ae483ed48

Comment 28

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/693eaa9c0e3b

Comment 29

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/4fed38d9046c

Comment 30

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/0b18989c61ba

Comment 31

[Ciprian Georgiu, Desktop QA]

I have reproduced this issue on an affected Nightly build (2024-07-22), with STR from comment 15.

The issue is verified as fixed on the latest builds available: latest Nightly 130.0a1, RC 129.0, Esr 128.1.0 and Esr 115.14.0. Tested with Win 11, macOS 14 and Ubuntu 22.04.

Comment 32

[Tyson Smith [:tsmith]]

Attachment: advisory.txt