Closed Bug 1644076 (CVE-2020-15648) Opened 1 year ago Closed 1 year ago

Bypass x-frame-options and csp frame-ancestors with `<object data=url>` and `<embed src=url>`

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla79
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 78+ verified
firefox77 --- wontfix
firefox78 + verified
firefox79 + verified

People

(Reporter: freddy, Assigned: ckerschb)

References

(Regression)

Details

(Keywords: regression, sec-moderate, Whiteboard: [domsecurity-active][post-critsmash-triage])

Attachments

(2 files)

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.

Summary: Bypass x-frame-options and csp frame-ancestors with `<object data=url>` and `<embed src=url`> → Bypass x-frame-options and csp frame-ancestors with `<object data=url>` and `<embed src=url>`

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...

Keywords: sec-moderatesec-low

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

Has Regression Range: --- → yes
Regressed by: 1599131
Duplicate of this bug: 1647092

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

Flags: needinfo?(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?

Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Flags: needinfo?(ckerschb) → needinfo?(matt.woodrow)
Priority: -- → P1
Whiteboard: [domsecurity-active]
Duplicate of this bug: 1646984

<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.

Flags: needinfo?(matt.woodrow)

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(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!

Severity: -- → S1
Flags: needinfo?(ckerschb)

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.

Keywords: sec-lowsec-moderate
Flags: qe-verify+
Whiteboard: [domsecurity-active] → [domsecurity-active][post-critsmash-triage]
Flags: needinfo?(jcristau)

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

Flags: needinfo?(jcristau) → in-testsuite+

(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!

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

Flags: qe-verify+

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

Flags: needinfo?(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
Flags: needinfo?(ckerschb)
Attachment #9159337 - Flags: approval-mozilla-release?
Attachment #9159337 - Flags: approval-mozilla-esr78?
Attachment #9159401 - Flags: approval-mozilla-esr78? approval-mozilla-release?

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

Attachment #9159337 - Flags: approval-mozilla-release?
Attachment #9159337 - Flags: approval-mozilla-release+
Attachment #9159337 - Flags: approval-mozilla-esr78?
Attachment #9159337 - Flags: approval-mozilla-esr78+
Attachment #9159401 - Flags: approval-mozilla-release?
Attachment #9159401 - Flags: approval-mozilla-release+
Attachment #9159401 - Flags: approval-mozilla-esr78?
Attachment #9159401 - Flags: approval-mozilla-esr78+

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

Status: RESOLVED → VERIFIED
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.