Closed Bug 1771685 (CVE-2022-38473) Opened 2 years ago Closed 2 years ago

Cross-origin frames can obtain top-level permissions b/c XSLT transform resets FeaturePolicy

Categories

(Core :: XSLT, defect, P2)

defect

Tracking

()

VERIFIED FIXED
105 Branch
Tracking Status
firefox-esr91 104+ verified
firefox-esr102 104+ verified
firefox101 --- wontfix
firefox102 --- wontfix
firefox103 --- wontfix
firefox104 + verified
firefox105 + verified
firefox106 --- verified

People

(Reporter: arminius, Assigned: peterv)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [adv-main104+][adv-esr91.13+][adv-esr102.2+])

Attachments

(5 files, 2 obsolete files)

Cross-origin children can obtain top-level permissions using XSLT because transformation effectively resets the document's FeaturePolicy.

Impact

The attack potential is very similar to bug 1755081. An untrusted cross-origin <iframe> may hijack the top-level origin's permissions to access media devices like the webcam, or prompt for permissions in the top-level origin's name.

Details

During XSL transformation, several properties of the source document are transferred to the result document in URIUtils::ResetWithSource(). Currently this doesn't consider the document's FeaturePolicy.

However, it's necessary to explicitly set the correct FP here, because the result document never runs Document::StartDocumentLoad() => Document::InitFeaturePolicy() to set the policy itself. Hence, the FP remains at its permissive default which, in a nested context (viz. cross-origin iframe), is equivalent to a blanket delegation of all permissions by the top-level context.

Proof of concept

Load the attached PoC in a cross-origin frame. Click the buttons to demo microphone/fullscreen access. Note that parent doesn't require HTTPS, but the PoC itself needs to be served securely so that navigator.mediaDevices is available.

The fullscreen case even works if HTML-sandboxed via e.g.:

<iframe sandbox="allow-scripts" src="https://untrusted.example/poc-with-acao-header.xml"></iframe>
See Also: → CVE-2022-29909
Group: core-security → dom-core-security

Here's a mochitest which asserts that an XSL-transformed document has the same FeaturePolicy as before.

Not sure what's the correct approach, but here is just a very rough patch idea to transfer the FP state to the new document.

diff --git a/dom/xslt/base/txURIUtils.cpp b/dom/xslt/base/txURIUtils.cpp
--- a/dom/xslt/base/txURIUtils.cpp
+++ b/dom/xslt/base/txURIUtils.cpp
@@ -9,6 +9,7 @@
 #include "nsIPrincipal.h"
 #include "mozilla/LoadInfo.h"
 #include "mozilla/dom/nsCSPContext.h"
+#include "mozilla/dom/FeaturePolicy.h"
 
 using mozilla::dom::Document;
 using mozilla::net::LoadInfo;
@@ -73,6 +74,21 @@ void URIUtils::ResetWithSource(Document*
   aNewDoc->SetReferrerInfo(sourceDoc->GetReferrerInfo());
   aNewDoc->SetEmbedderPolicy(sourceDoc->GetEmbedderPolicy());
 
+  mozilla::dom::FeaturePolicy* sourceFP = sourceDoc->FeaturePolicy();
+  mozilla::dom::FeaturePolicy* newFP = aNewDoc->FeaturePolicy();
+  newFP->SetInheritedDeniedFeatureNames(
+      sourceFP->InheritedDeniedFeatureNames());
+
+  const auto& declaredString = sourceFP->DeclaredString();
+  if (sourceFP->GetSelfOrigin() && !declaredString.IsEmpty()) {
+    newFP->SetDeclaredPolicy(sourceDoc, sourceFP->DeclaredString(),
+                             sourceFP->GetSelfOrigin(),
+                             sourceFP->GetSrcOrigin());
+  }
+  for (auto& featureName : sourceFP->AttributeEnabledFeatureNames()) {
+    newFP->MaybeSetAllowedPolicy(featureName);
+  }
+
   // Inherit the csp if there is one
   nsCOMPtr<nsIContentSecurityPolicy> csp = sourceDoc->GetCsp();
   if (csp) {

That's essentially the approach taken to sync the FP over IPC from here: https://searchfox.org/mozilla-central/rev/7751fef9eeb3db0a07ae4680daa2a62bd8f49882/dom/security/featurepolicy/FeaturePolicyUtils.cpp#255-264,291-302

Christoph: what other security policies do we need to worry might not be carried over into a generated document if we're getting this one wrong?

Flags: needinfo?(ckerschb)

(In reply to Daniel Veditz [:dveditz] from comment #3)

Christoph: what other security policies do we need to worry might not be carried over into a generated document if we're getting this one wrong?

I guess it's best to look at docshell code whenever we reload a document, or also create an about:blank document.

ReloadDocument might be a good starting point to look what we copy over, because it's not only policies, but also base-uri, triggeringPrincipal, sandbox-flags, ...

We should really have a container-struct which simplifies copying things over. It's on our roadmap and we have Bug 1739442 on file because we consistently run into this problem.

Flags: needinfo?(ckerschb)

Peter is working on this.

Assignee: nobody → peterv

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(continuation)
Severity: -- → S2
Flags: needinfo?(continuation)
Priority: -- → P2
Summary: Cross-origin frames can obtain top-level permissions b/c XSL transform resets FeaturePolicy → Cross-origin frames can obtain top-level permissions b/c XSLT transform resets FeaturePolicy

Depends on D151516

Attachment #9281029 - Attachment is obsolete: true

Comment on attachment 9284953 [details]
Bug 1771685 - Init feature policy. r?farre!

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Patch makes it fairly obvious that it's related to XSLT and feature policy.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which older supported branches are affected by this flaw?: All
  • 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?: Should just apply.
  • How likely is this patch to cause regressions; how much testing does it need?: Fairly safe, makes sure we apply the feature policy restrictions for XSLT documents.
  • Is Android affected?: Yes
Attachment #9284953 - Flags: sec-approval?
Attachment #9284954 - Flags: sec-approval?

Comment on attachment 9284953 [details]
Bug 1771685 - Init feature policy. r?farre!

Approved to land and uplift

Attachment #9284953 - Flags: sec-approval?
Attachment #9284953 - Flags: sec-approval+
Attachment #9284953 - Flags: approval-mozilla-esr91+
Attachment #9284953 - Flags: approval-mozilla-esr102+
Attachment #9284953 - Flags: approval-mozilla-beta+

Comment on attachment 9284954 [details]
Bug 1771685 - Testcase. r?farre!

Can land the test case on/after Oct 5th

Attachment #9284954 - Flags: sec-approval?
Whiteboard: [reminder-test 2022-10-15]
Whiteboard: [reminder-test 2022-10-15] → [reminder-test 2022-10-5]

Peter's out for a bit, so I'll try to do the landing and uplift requests.

Group: dom-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit auto_nag documentation.

Flags: sec-bounty?
See Also: → CVE-2022-40959
Flags: sec-bounty? → sec-bounty+
Whiteboard: [reminder-test 2022-10-5] → [reminder-test 2022-10-5][adv-main104+]
Attached file advisory.txt (obsolete) —
Attached file advisory.txt
Attachment #9290144 - Attachment is obsolete: true
Whiteboard: [reminder-test 2022-10-5][adv-main104+] → [reminder-test 2022-10-5][adv-main104+][adv-esr91.13+]
Whiteboard: [reminder-test 2022-10-5][adv-main104+][adv-esr91.13+] → [reminder-test 2022-10-5][adv-main104+][adv-esr91.13+][adv-esr102.2+]
Alias: CVE-2022-38473
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Flags: qe-verify- → qe-verify+
QA Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][qa-triaged]

This issue is Verified as fixed in our latest 91.13.0esr, 102.3.0esr, 104.0.2, 105.0b7 and 106.0a1 (2022-09-04).

QA Whiteboard: [post-critsmash-triage][qa-triaged] → [post-critsmash-triage]
Group: core-security-release

5 months ago, Tom Ritter [:tjr] placed a reminder on the bug using the whiteboard tag [reminder-test 2022-10-5] .

peterv, please refer to the original comment to better understand the reason for the reminder.

Flags: needinfo?(peterv)
Whiteboard: [reminder-test 2022-10-5][adv-main104+][adv-esr91.13+][adv-esr102.2+] → [adv-main104+][adv-esr91.13+][adv-esr102.2+]
Flags: needinfo?(peterv)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: