Bug 1612378 - Part 9: Make StorageAccess::InternalStorageAllowedCheck() to aslo checks the document's URI when doing a about URI check. r=dimi,baku
authorTim Huang <tihuang@mozilla.com>
Wed, 25 Mar 2020 13:22:11 +0000
changeset 520374 50d282ea77f1381e5bfa7f1297848648084239a8
parent 520373 d9ac725f5a57dfd7c5b7d1aa6f6580357529720c
child 520375 a147f4b085f22af6d1673aecf65d26a49af6202e
push id37249
push userdvarga@mozilla.com
push dateWed, 25 Mar 2020 21:39:06 +0000
treeherdermozilla-central@b3c3f7d0f044 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdimi, baku
bugs1612378
milestone76.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1612378 - Part 9: Make StorageAccess::InternalStorageAllowedCheck() to aslo checks the document's URI when doing a about URI check. r=dimi,baku So far, we haven't checked the window's document uri when doing a about URI check in StorageAccess::InternalStorageAllowedCheck(). This brought an issue that if the checked page is an about page with the system principal, for example the 'about:devtools-toolbox' we will miss this check. Because of the system principal doesn't have a URI. So we need to check the document URI instead to know if this is a about page. Differential Revision: https://phabricator.services.mozilla.com/D66481
toolkit/components/antitracking/StorageAccess.cpp
--- a/toolkit/components/antitracking/StorageAccess.cpp
+++ b/toolkit/components/antitracking/StorageAccess.cpp
@@ -78,27 +78,31 @@ static StorageAccess InternalStorageAllo
   StorageAccess access = StorageAccess::eAllow;
 
   // We don't allow storage on the null principal, in general. Even if the
   // calling context is chrome.
   if (aPrincipal->GetIsNullPrincipal()) {
     return StorageAccess::eDeny;
   }
 
+  nsCOMPtr<nsIURI> documentURI;
   if (aWindow) {
     // If the document is sandboxed, then it is not permitted to use storage
     Document* document = aWindow->GetExtantDoc();
     if (document && document->GetSandboxFlags() & SANDBOXED_ORIGIN) {
       return StorageAccess::eDeny;
     }
 
     // Check if we are in private browsing, and record that fact
     if (nsContentUtils::IsInPrivateBrowsing(document)) {
       access = StorageAccess::ePrivateBrowsing;
     }
+
+    // Get the document URI for the below about: URI check.
+    documentURI = document ? document->GetDocumentURI() : nullptr;
   }
 
   uint32_t lifetimePolicy;
 
   // WebExtensions principals always get BEHAVIOR_ACCEPT as cookieBehavior
   // and ACCEPT_NORMALLY as lifetimePolicy (See Bug 1406675 for rationale).
   auto policy = BasePrincipal::Cast(aPrincipal)->AddonPolicy();
 
@@ -136,17 +140,23 @@ static StorageAccess InternalStorageAllo
   // IndexedDB: allowed in 3rd-party iframes by default. Preference can be set
   //   to disable in 3rd-party, which will disallow in about: URIs, unless they
   //   are within a specific whitelist.
   //
   // This means that behavior for storage with internal about: URIs should not
   // be affected, which is desireable due to the lack of automated testing for
   // about: URIs with these preferences set, and the importance of the correct
   // functioning of these URIs even with custom preferences.
-  if ((aURI && aURI->SchemeIs("about")) || aPrincipal->SchemeIs("about")) {
+  //
+  // We need to check the aURI or the document URI here instead of only checking
+  // the URI from the principal. Because the principal might not have a URI if
+  // it is a system principal.
+  if ((aURI && aURI->SchemeIs("about")) ||
+      (documentURI && documentURI->SchemeIs("about")) ||
+      aPrincipal->SchemeIs("about")) {
     return access;
   }
 
   if (!StorageDisabledByAntiTracking(aWindow, aChannel, aPrincipal, aURI,
                                      aRejectedReason)) {
     return access;
   }