Bug 1415352: Part 5b - Use the last component principal as principal to inherit for data: URLs. r=bz,krizsa
authorKris Maglione <maglione.k@gmail.com>
Wed, 22 Nov 2017 14:20:26 -0800
changeset 437773 96ffd32355aecf7d2999ff767c8ca09abb6f3ea0
parent 437772 0f044857350891d27712d3be932bf1ae10f4e5ce
child 437774 d811ce4ebcd9a7bfce3e6ac0ec5d006f9e2ff82c
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersbz, krizsa
bugs1415352
milestone59.0a1
Bug 1415352: Part 5b - Use the last component principal as principal to inherit for data: URLs. r=bz,krizsa The logic for choosing the principal here was originally written before loadInfo had a separate principalToInherit field, and we needed to specify it via the triggeringPrincipal instead. At that point, we had to choose a component principal with permission to load the URI at the start of the request. However, now that we have a separate field for the principal to inherit, it's only needed after access checks have passed and we know that we have a URI which inherits a principal. In that case, the current logic causes us to always inherit the first principal in the whitelist (which is the page principal) for URIs (such as data: URIs) that always inherit, where we really want to inherit the last (which is the extension principal). MozReview-Commit-ID: EPoUNuOCwrH
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/ExpandedPrincipal.cpp
caps/ExpandedPrincipal.h
netwerk/base/LoadInfo.cpp
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -354,22 +354,20 @@ BasePrincipal::AddonHasPermission(const 
 {
   if (auto policy = AddonPolicy()) {
     return policy->HasPermission(aPerm);
   }
   return false;
 }
 
 nsIPrincipal*
-BasePrincipal::PrincipalToInherit(nsIURI* aRequestedURI,
-                                  bool aAllowIfInheritsPrincipal)
+BasePrincipal::PrincipalToInherit(nsIURI* aRequestedURI)
 {
   if (Is<ExpandedPrincipal>()) {
-    return As<ExpandedPrincipal>()->PrincipalToInherit(aRequestedURI,
-                                                       aAllowIfInheritsPrincipal);
+    return As<ExpandedPrincipal>()->PrincipalToInherit(aRequestedURI);
   }
   return this;
 }
 
 already_AddRefed<BasePrincipal>
 BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI,
                                        const OriginAttributes& aAttrs)
 {
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -126,21 +126,17 @@ public:
   inline bool FastSubsumesConsideringDomainIgnoringFPD(nsIPrincipal* aOther);
 
   // Returns the principal to inherit when a caller with this principal loads
   // the given URI.
   //
   // For most principal types, this returns the principal itself. For expanded
   // principals, it returns the first sub-principal which subsumes the given URI
   // (or, if no URI is given, the last whitelist principal).
-  //
-  // The aAllowIfInheritsPrincipal argument is passed through to CheckMayLoad()
-  // to determine which consistituent principals may load the requested URI.
-  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr,
-                                   bool aAllowIfInheritsPrincipal = true);
+  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr);
 
   /**
    * Returns true if this principal's CSP should override a document's CSP for
    * loads that it triggers. Currently true only for expanded principals which
    * subsume the document principal, and add-on codebase principals regardless
    * of whether they subsume the document principal.
    */
   bool OverridesCSP(nsIPrincipal* aDocumentPrincipal)
--- a/caps/ExpandedPrincipal.cpp
+++ b/caps/ExpandedPrincipal.cpp
@@ -176,23 +176,27 @@ ExpandedPrincipal::AddonHasPermission(co
     if (BasePrincipal::Cast(mPrincipals[i])->AddonHasPermission(aPerm)) {
       return true;
     }
   }
   return false;
 }
 
 nsIPrincipal*
-ExpandedPrincipal::PrincipalToInherit(nsIURI* aRequestedURI,
-                                      bool aAllowIfInheritsPrincipal)
+ExpandedPrincipal::PrincipalToInherit(nsIURI* aRequestedURI)
 {
   if (aRequestedURI) {
+    // If a given sub-principal subsumes the given URI, use that principal for
+    // inheritance. In general, this only happens with certain CORS modes, loads
+    // with forced principal inheritance, and creation of XML documents from
+    // XMLHttpRequests or fetch requests. For URIs that normally inherit a
+    // principal (such as data: URIs), we fall back to the last principal in the
+    // whitelist.
     for (const auto& principal : mPrincipals) {
-      if (NS_SUCCEEDED(principal->CheckMayLoad(aRequestedURI, false,
-                                               aAllowIfInheritsPrincipal))) {
+      if (Cast(principal)->MayLoadInternal(aRequestedURI)) {
         return principal;
       }
     }
   }
   return mPrincipals.LastElement();
 }
 
 nsresult
--- a/caps/ExpandedPrincipal.h
+++ b/caps/ExpandedPrincipal.h
@@ -34,18 +34,17 @@ public:
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   NS_IMETHOD GetAddonId(nsAString& aAddonId) override;
   virtual bool AddonHasPermission(const nsAtom* aPerm) override;
   virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
   // Returns the principal to inherit when this principal requests the given
   // URL. See BasePrincipal::PrincipalToInherit.
-  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr,
-                                   bool aAllowIfInheritsPrincipal = true);
+  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr);
 
 protected:
   explicit ExpandedPrincipal(nsTArray<nsCOMPtr<nsIPrincipal>> &aWhiteList);
 
   virtual ~ExpandedPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther,
                         DocumentDomainConsideration aConsideration) override;
--- a/netwerk/base/LoadInfo.cpp
+++ b/netwerk/base/LoadInfo.cpp
@@ -536,22 +536,18 @@ LoadInfo::FindPrincipalToInherit(nsIChan
     return mPrincipalToInherit;
   }
 
   nsCOMPtr<nsIURI> uri = mResultPrincipalURI;
   if (!uri) {
     Unused << aChannel->GetOriginalURI(getter_AddRefs(uri));
   }
 
-  bool dataInherits = mSecurityFlags & (SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS |
-                                        SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
-                                        SEC_REQUIRE_CORS_DATA_INHERITS);
-
   auto prin = BasePrincipal::Cast(mTriggeringPrincipal);
-  return prin->PrincipalToInherit(uri, dataInherits);
+  return prin->PrincipalToInherit(uri);
 }
 
 NS_IMETHODIMP
 LoadInfo::GetSandboxedLoadingPrincipal(nsIPrincipal** aPrincipal)
 {
   if (!(mSecurityFlags & nsILoadInfo::SEC_SANDBOXED)) {
     *aPrincipal = nullptr;
     return NS_OK;