1644076 - (CVE-2020-15648) Bypass x-frame-options and csp frame-ancestors with `<object data=url>` and `<embed src=url>`

Status: VERIFIED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Frederik Braun [:freddy]]

Commonly, websites use a response header like X-Frame-Options: DENY or CSP's frame-ancestors directive to disallow being embedded across origins.

It seems that this restriction can be easily bypassed with embeds like <object data=url> or <embed src=url>.

STR:

1. Find a website that disallows framing (i.e., X-Frame-Options set to DENY or SAMEORIGIN or has a CSP directive that does the same). This example will use https://accounts.google.com
2. Embed on a different origin using embed or object elements. Example:
   data:text/html,embed:<embed src="https://accounts.google.com"><hr>object:<object data="https://accounts.google.com">
3. Observe how both frames are loading successfully.

Expected results: Frames should not load and show an error page.

Comment 1

[Frederik Braun [:freddy]]

Interestingly, the test doesn't work when hosted on the web (i.e.., with content principal). Maybe there's a problem with the nullprincipal that the data URL gets? I wonder what happens with iframe sandbox...

Comment 2

[Frederik Braun [:freddy]]

34:07.57 INFO: Last good revision: 2e73e6800f7e6009363e67bd59aa87d34c27967f
34:07.57 INFO: First bad revision: ba5511bfb76bd6dcd168e463fc51926c11fc4c60
34:07.57 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2e73e6800f7e6009363e67bd59aa87d34c27967f&tochange=ba5511bfb76bd6dcd168e463fc51926c11fc4c60

Pointing to Bug 1599131

Comment 3

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

Set release status flags based on info from the regressing bug 1599131

Comment 5

[:Gijs (he/him)]

We've had a third-party report of this (see dupe). Is there any chance of fixing this in time for 78/79?

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

Yes we should definitely fix that.

@Matt: Initially XFO checks were performed within Document::StartDocumentLoad. Within Bug 1599131 we moved them to DocumentLoadListener::OnStartRequest.

It seems that DocumentLoadListener::OnStartRequest does not get called for <embed src="https://accounts.google.com"> or also <object data="https://accounts.google.com">.

I know we have been discussing where to add the XFO checks before, but given that, can you think of a better place to add the XFO checks?

Comment 8

[Matt Woodrow (:mattwoodrow)]

<object>/<embed> don't go through DocumentChannel/DocumentLoadListener right now unfortunately.

This is being worked on right now in bug 1646899, though it might not be able to land before soft freeze tomorrow.

If we want something to uplift, then reverting the object-specific paths of bug 1599131 might be the safest choice.

Comment 9

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

The severity field is not set for this bug.
:ckerschb, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #9)

The severity field is not set for this bug.

I am on it!

Comment 11

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1644076: Bring back XFO check in StartDocumentLoad for type internal object and embed. r=smaug

Comment 12

[Christoph Kerschbaumer [:ckerschb]]

Attachment: Bug 1644076: Test XFO for type embed and object. r=smaug

Comment 13

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

https://hg.mozilla.org/integration/autoland/rev/45b9fcc2861739282916ccf3e4eef0ae234361ba
https://hg.mozilla.org/integration/autoland/rev/3adb94b0b3ffcb65f9c8d982b80c2a491678b5af
https://hg.mozilla.org/mozilla-central/rev/45b9fcc28617
https://hg.mozilla.org/mozilla-central/rev/3adb94b0b3ff

Comment 14

[Frederik Braun [:freddy]]

Bumping security rating. This isn't directly bad for Firefox's security, but lots of websites rely on us to implement XFO properly for their security.

Comment 17

[Julien Cristau [:jcristau]]

No time for 78.0, but I've put this on the list for 78.0.1.

Comment 18

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Julien Cristau [:jcristau] from comment #17)

No time for 78.0, but I've put this on the list for 78.0.1.

Thanks!

Comment 19

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Beta Build 79.0b2 on Windows 10. I will update the flags for it.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Release and ESR78 approval when you get a chance.

Comment 21

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 9159337 [details]
Bug 1644076: Bring back XFO check in StartDocumentLoad for type internal object and embed. r=smaug

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: It's a sec moderate from a Mozilla's point of view, though this web application security mechanism to prevent clickjacking might be considered a sec-high for lots of web pages since XFO is the number one security mechanism against clickjacking.
• User impact if declined: Web pages (using logins or other forms) might be framed by malicious actors and tricked into revealing their credentials to the attacker.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Before making XFO fission compliant we have been shipping this code for a very long time - we are basically reverting to that behavior.
• String or UUID changes made by this patch: no

Beta/Release Uplift Approval Request

• User impact if declined: It's a sec moderate from a Mozilla's point of view, though this web application security mechanism to prevent clickjacking might be considered a sec-high for lots of web pages since XFO is the number one security mechanism against clickjacking.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Before making XFO fission compliant we have been shipping this code for a very long time - we are basically reverting to that behavior.
• String changes made/needed: no

Comment 22

[Julien Cristau [:jcristau]]

Comment on attachment 9159337 [details]
Bug 1644076: Bring back XFO check in StartDocumentLoad for type internal object and embed. r=smaug

approved for 78.0.2

Comment 23

[Julien Cristau [:jcristau]]

for 78.0.2:
https://hg.mozilla.org/releases/mozilla-release/rev/0f24ccda487488b68cc4b8d019099385822beb4e
https://hg.mozilla.org/releases/mozilla-release/rev/64121223b74e5e3f4758377f92ea25730b80e74a

for 78.0.2esr:
https://hg.mozilla.org/releases/mozilla-esr78/rev/a3461bab911360a7827bf84983868e49a02c0681
https://hg.mozilla.org/releases/mozilla-esr78/rev/55b39062d69b8043791c674991a365afcfe05263

for 78.1esr:
https://hg.mozilla.org/releases/mozilla-esr78/rev/dde9b5fe960f2075bc678bc55ca7ef769ce4ebac
https://hg.mozilla.org/releases/mozilla-esr78/rev/1777c4095b0b2ef909a6e391db021d2b27235d5e

Comment 24

[Rares Doghi, Desktop QA]

This issue is Verified as fixed in Firefox 78.1.0esr/78.0.2esr as well as Release 78.0.2 on Windows 10.