Closed Bug 1738418 (CVE-2021-43543) Opened 11 months ago Closed 10 months ago

<embed/object> frames bypass parent's CSP `sandbox` header directive

Categories

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

defect

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox-esr91 95+ fixed
firefox94 --- wontfix
firefox95 + fixed
firefox96 --- fixed

People

(Reporter: arminius, Assigned: freddy)

Details

(Keywords: regression, regressionwindow-wanted, sec-moderate, Whiteboard: [domsecurity-active][post-critsmash-triage][adv-main95+][adv-ESR91.4.0+])

Attachments

(4 files)

A given Content-Security-Policy: sandbox ...; HTTP header directive is not correctly enforced for <embed> and <object> child frames during load, allowing to bypass the set restrictions.

Test case

parent.html:

Content-Security-Policy: sandbox

<iframe src="child.html"></iframe>
<embed src="child.html"></embed>
<object data="child.html"></object>

child.html:

<script>prompt(document.domain)</script>

Expected result

Per the blanket sandbox directive, all three child frames should be treated as cross-origin loads and be denied script execution.

Actual result

The <embed> and <object> children are able to execute same-origin script code.

CSP3 spec reference:

6.2.2. sandbox
The sandbox directive specifies an HTML sandbox policy which the user agent will apply to a resource, just as though it had been included in an iframe with a sandbox property.

I will follow up with additional details to the root cause.

Flags: sec-bounty?

From what I understand, an object/embed load has to be handled different from an iframe load in a few places because it's not necessarily going to load a document. So for an iframe document load, the LoadInfo is created in DocumentLoadListener::OpenDocument() via

  RefPtr<CanonicalBrowsingContext> browsingContext =
      GetDocumentBrowsingContext();
...
  RefPtr<LoadInfo> loadInfo =
      CreateDocumentLoadInfo(browsingContext, aLoadState);

and

static auto CreateDocumentLoadInfo(CanonicalBrowsingContext* aBrowsingContext,
                                   nsDocShellLoadState* aLoadState)
    -> already_AddRefed<LoadInfo> {
  uint32_t sandboxFlags = aBrowsingContext->GetSandboxFlags();
  RefPtr<LoadInfo> loadInfo;

  auto securityFlags = SecurityFlagsForLoadInfo(aLoadState);

  if (aBrowsingContext->GetParent()) {
    loadInfo = LoadInfo::CreateForFrame(aBrowsingContext,
                                        aLoadState->TriggeringPrincipal(),
                                        securityFlags, sandboxFlags);
...

Here, the sandboxFlags that scripts will be checked against are read from the BC for the embedded subdocument. So in the test case above, the iframe is loaded with the right flags.

Now for object loads, the LoadInfo is constructed via DocumentLoadListener::OpenObject():

  auto sandboxFlags = GetLoadingBrowsingContext()->GetSandboxFlags();

  RefPtr<LoadInfo> loadInfo = CreateObjectLoadInfo(
      aLoadState, aInnerWindowId, aContentPolicyType, sandboxFlags);
...

This takes the loading BC's sandboxFlags. In the test case, that would be the top-level BC which doesn't have any sandbox flags set. It seems that's how the restrictions get dropped here.

Here's an additional mochitest to demonstrate the bug.

0:02.94 TEST_START: dom/security/test/csp/test_bug1738418.html
 0:04.00 FAIL document in <object> should have sandboxed origin - got "mochi.test", expected ""
    SimpleTest.is@SimpleTest/SimpleTest.js:500:14
    @dom/security/test/csp/test_bug1738418.html:17:5
 0:04.00 FAIL document in <embed> should have sandboxed origin - got "mochi.test", expected ""
    SimpleTest.is@SimpleTest/SimpleTest.js:500:14
    @dom/security/test/csp/test_bug1738418.html:17:5
 0:04.00 PASS document in <iframe> should have sandboxed origin
 0:04.01 TEST_END: Test OK. Subtests passed 1/3. Unexpected 2

Christoph, can you make sure this ends up in front of the right people?

Group: core-security → dom-core-security
Flags: needinfo?(ckerschb)
Component: DOM: Navigation → DOM: Security

Freddy: can you take this one on?

Assignee: nobody → fbraun
Severity: -- → S2
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ckerschb) → needinfo?(fbraun)
Keywords: sec-moderate
Priority: -- → P2
Whiteboard: [domsecurity-active]

Wow, thanks so much for the detailed description and testcase, that's super useful!
I'm taking a look.

Flags: needinfo?(fbraun)
Flags: sec-bounty? → sec-bounty+

A likely regression is bug 164689. But with all previously involved people (patch author and reviewers) having left Mozilla, I'm taking a wild guess for reviews.

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

A likely regression is bug 164689.

That looks very unlikely. Does the bug number perhaps miss one digit?

bug 1646899 might be the right one.

Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch

test needs landing too. do we need to wait?

Flags: in-testsuite?

The patch landed in nightly and beta is affected.
:freddy, 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?(fbraun)

Test should wait, yes. Please move ahead with the Beta & ESR91 requests.

Our last beta builds in a few hours, do we want to uplift that in 95 beta and ESR91 this cycle?

Comment on attachment 9250343 [details]
Bug 1738418 - set sandboxflags for object/embed loads correctly r?ckerschb,smaug

Beta/Release Uplift Approval Request

  • User impact if declined: Our Content Security implementation correctly applies provided sandbox attribute to loads of type iframe but not to loads of type embed and object which in turn grants JS code in embed and object to run with same-origin privileges.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • 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): Risk is low since we are only propagating the missed sandbox flags to loads of type embed and object. (Every page not providing a CSP sandbox attribute is unaffected).
  • String changes made/needed: no

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: As above.
  • User impact if declined:
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String or UUID changes made by this patch:
Attachment #9250343 - Flags: approval-mozilla-esr91?
Attachment #9250343 - Flags: approval-mozilla-beta?
Attachment #9248526 - Flags: approval-mozilla-beta?
Attachment #9250342 - Flags: approval-mozilla-beta? approval-mozilla-esr91?

Comment on attachment 9248526 [details] [diff] [review]
testcase_mochitest.patch

This file does not need to be uplifted.

Attachment #9248526 - Flags: approval-mozilla-beta?

Comment on attachment 9250343 [details]
Bug 1738418 - set sandboxflags for object/embed loads correctly r?ckerschb,smaug

Approved for landing on the beta95 and esr91.4 branches, thanks!

Attachment #9250343 - Flags: approval-mozilla-esr91?
Attachment #9250343 - Flags: approval-mozilla-esr91+
Attachment #9250343 - Flags: approval-mozilla-beta?
Attachment #9250343 - Flags: approval-mozilla-beta+

I am leaving the uplifting of the testcase to next cycle, once it has also landed on mozilla-central.

Flags: qe-verify-
Whiteboard: [domsecurity-active] → [domsecurity-active][post-critsmash-triage]

Hey, thank you for handling the patch uplift in my absence.

Flags: needinfo?(fbraun)
Whiteboard: [domsecurity-active][post-critsmash-triage] → [domsecurity-active][post-critsmash-triage][adv-main95+][adv-ESR91.4.0+]
Attached file advisory.txt
Alias: CVE-2021-43543
Attachment #9250342 - Flags: approval-mozilla-esr91?
Attachment #9250342 - Flags: approval-mozilla-beta?
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.