Closed Bug 1744352 (CVE-2022-26384) Opened 3 years ago Closed 2 years ago

Sandboxed iFrame XSS - javascript URI's run with target _blank

Categories

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

defect

Tracking

()

VERIFIED FIXED
99 Branch
Tracking Status
firefox-esr91 98+ verified
firefox97 --- wontfix
firefox98 + verified
firefox99 + verified

People

(Reporter: emcmanus, Assigned: nika)

References

()

Details

(Keywords: csectype-sop, reporter-external, sec-high, Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey][adv-main98+][adv-esr91.7+])

Attachments

(4 files, 1 obsolete file)

(Thank you Firefox developers for all your hard work! The world needs Firefox.)

I believe the following is an XSS vulnerability present in the current release channel. By specifying target="blank" you can execute javascript in the domain of a sandboxed iframe, even though allow-scripts is not specified.

This behavior is blocked by Edge, Chrome, and Safari. Demo: https://emcmanus.github.io/xss.html

Expected result:

  • Sandboxed iframes should not execute JS unless allow-scripts is specified
  • Firefox should block the javascript: uri
  • Firefox behavior should match other browsers

Actual result:

  • Javascript runs in the current domain, even though allow-scripts is not specified
  • Edge, Safari and Chrome block this behavior
  • It does not matter whether you use src or srcdoc

I believe a related bug was patched in Firefox 79: https://www.mozilla.org/en-US/security/advisories/mfsa2020-30/#CVE-2020-15653

Here is the source of the demo:

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv='content-type' content='text/html; charset=UTF-8'>
  </head>
  <body>
    <iframe sandbox="allow-same-origin allow-popups" srcdoc="<a href=&quot;javascript:alert(document.domain)&quot; target=&quot;_blank&quot;>click me</a>"></iframe>
  </body>
</html>
Flags: sec-bounty?
Group: firefox-core-security → core-security
Type: task → defect
Component: Security → DOM: Security
Product: Firefox → Core
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form]

Reminds me of bug 1738418, I'll take this.

Assignee: nobody → fbraun

I started looking at the patch from bug 1521542 and came to the conclusion that this is likely fixed in ContentChild::ProvideWindowCommon() too, but I also wanted to start with a test to ensure this is the right direction..

However, when looking at existing tests in dom/html/test. It's a bit tricky to make this work with calling ok() (or postMessage) on the opener, since the new, _blank context does not seem to have an opener?!

I'll keep looking.

Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

It's hard to tell simply by comparing to chrome behavior because they always block javascript: links targeted at a new window regardless of sandboxing.

Things that could be OK:

  • the sandbox has allow-same-origin (a running script being same-origin could be fine)
  • the sandbox allows popups

Things we get wrong:

  • the sandbox does NOT have allow-popups-to-escape-sandbox
  • the sandbox does NOT have allow-scripts

Javascript urls are weird creatures and this bug might just beunique to that case, but to be sure: do we have tests that ensure popups from a sandbox are themselves properly sandboxed and, with this collection of attributes, can't run scripts?

However, when looking at existing tests in dom/html/test. It's a bit tricky to make this work with calling ok() (or postMessage) on the opener, since the new, _blank context does not seem to have an opener?!

See bug 1522083, and also
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target
https://github.com/whatwg/html/issues/4078

You should be able to set an explicit rel="opener"

Group: core-security → dom-core-security
Component: DOM: Security → DOM: Core & HTML

do we have tests that ensure popups ...

And I guess we need to make sure window.open() and target=_blank are treated the same wrt inheriting the sandbox properties.

These cases should all be covered by WPT, except the case in this bug because we're the only browser engine left that opens javascript links in a new window so no point in writing cross-browser tests for it. (Maybe Internet Explorer still does, too?). Even Brendan's own browser doesn't try to bring that behavior back -- time to throw in the towel? Would certainly stop a bunch of unique-to-Firefox exploit attempts.

Severity: -- → S3

Nika has looked at this, and wrote a patch. I think she's trying to figure out exactly how to test it.

Assignee: fbraun → nika
Flags: needinfo?(nika)
Group: dom-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 99 Branch

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(nika)
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form] → [reporter-external] [client-bounty-form][post-critsmash-triage]
See Also: → CVE-2020-15653

We rated this "moderate" incorrectly (thinking of CSP bypasses), but sandboxing is more fundamentally and relied on more broadly on the web. raising to sec-high

Flags: sec-bounty? → sec-bounty+

This needs an uplift to beta and ESR. We're running out of betas (this might be the last week?), please do it soon.

Comment on attachment 9262937 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts, r=smaug

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Note, the patch applies to esr91 with --fuzz=3
  • User impact if declined: Executing javascript in the domain of a sandboxed iframe, even though allow-scripts is not specified
  • Fix Landed on Version: 99
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Passing around the flags from the original browsing context to the new one should be relatively safe.

Beta/Release Uplift Approval Request

  • User impact if declined:
  • 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):
  • String changes made/needed: NA
Attachment #9262937 - Flags: approval-mozilla-esr91?
Attachment #9262937 - Flags: approval-mozilla-beta?
Attachment #9262939 - Flags: approval-mozilla-beta?

Comment on attachment 9262937 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts, r=smaug

sorry, fuzz didn't do the right thing here after all

Attachment #9262937 - Flags: approval-mozilla-esr91?
Attachment #9262937 - Flags: approval-mozilla-beta?
Attachment #9262939 - Flags: approval-mozilla-beta?

Comment on attachment 9262937 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts, r=smaug

Beta/Release Uplift Approval Request

  • User impact if declined: Executing javascript in the domain of a sandboxed iframe, even though allow-scripts is not specified

(This patch applies cleanly to beta, but does not to esr)

  • 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): Passing around the flags from the original browsing context to the new one should be relatively safe.
  • String changes made/needed: NA
Attachment #9262937 - Flags: approval-mozilla-beta?
Attachment #9262939 - Flags: approval-mozilla-beta?

Comment on attachment 9265068 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts (for esr), r=mccr8

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Executing javascript in the domain of a sandboxed iframe, even though allow-scripts is not specified
  • Fix Landed on Version: 99
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Passing around the flags from the original browsing context to the new one should be relatively safe.
Attachment #9265068 - Flags: approval-mozilla-esr91?
Attachment #9262939 - Flags: approval-mozilla-esr91?
Flags: needinfo?(nika)

Comment on attachment 9262937 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts, r=smaug

Approved for 98 beta 9, last beta, thanks.

Attachment #9262937 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9262939 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Verified as fixed on Windows 10 x64, macOS 11.6 and on Ubuntu 20.04 x64.

Comparing the results on Microsoft Edge and Chrome where the new tab loaded from the demo provided in the description redirect the user to about:blank#blocked instead of an empty address bar provided on Firefox.
Is this intended? Are there any intentions to change that?
Thanks.

Flags: needinfo?(nika)

Maybe Olli can answer the question sooner than Nika.

Flags: needinfo?(bugs)

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.

Flags: needinfo?(nika)
Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage] → [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey]

We don't show blocked, no.
Could you file a followup bug to do so.

Flags: needinfo?(bugs)

Comment on attachment 9265068 [details]
Bug 1744352 - Part 1: Ensure we set InitialSandboxFlags when opening or replacing toplevel BrowsingContexts (for esr), r=mccr8

Approved for 91.7esr.

Attachment #9265068 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
Attachment #9262939 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+

(In reply to Olli Pettay [:smaug] from comment #24)

We don't show blocked, no.
Could you file a followup bug to do so.

Logged bug 1756999.
Thanks.

See Also: → 1756999

Should we consider the new bug a security issue also?
Thanks.

Flags: needinfo?(bugs)

That isn't a security issue, just somewhat minor UI issue, IMO.

Flags: needinfo?(bugs)
Flags: needinfo?(nika)

Verified as fixed on Windows 10 x64, macOS 11.6 and on Ubuntu 20.04 x64.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey] → [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey][adv-main98+]
Attached file advisory.txt (obsolete) —
Whiteboard: [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey][adv-main98+] → [reporter-external] [client-bounty-form][post-critsmash-triage][sec-survey][adv-main98+][adv-esr91.7+]
Attached file advisory.txt
Attachment #9266088 - Attachment is obsolete: true
Alias: CVE-2022-26384
See Also: → CVE-2022-29911
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: