Bug 956382 - Hoist nsScriptSecurityManager::CheckSameOriginPrincipal into nsPrincipal::EqualsConsideringDomain. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 13 Feb 2014 18:57:36 -0800
changeset 185931 d8de4338ba02b092da9d0db0be091445e570d4fd
parent 185930 ce0bc42b933d307befb0cdccbee80ab2831f7a1a
child 185932 18d581a3cdda20a54de51540ac7e7fa99144e7ad
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs956382
milestone30.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 956382 - Hoist nsScriptSecurityManager::CheckSameOriginPrincipal into nsPrincipal::EqualsConsideringDomain. r=mrbkap
caps/include/nsScriptSecurityManager.h
caps/src/nsPrincipal.cpp
caps/src/nsScriptSecurityManager.cpp
--- a/caps/include/nsScriptSecurityManager.h
+++ b/caps/include/nsScriptSecurityManager.h
@@ -70,19 +70,16 @@ public:
      */
     static bool SecurityCompareURIs(nsIURI* aSourceURI, nsIURI* aTargetURI);
     static uint32_t SecurityHashURI(nsIURI* aURI);
 
     static nsresult 
     ReportError(JSContext* cx, const nsAString& messageTag,
                 nsIURI* aSource, nsIURI* aTarget);
 
-    static nsresult
-    CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                             nsIPrincipal* aObject);
     static uint32_t
     HashPrincipalByOrigin(nsIPrincipal* aPrincipal);
 
     static bool
     GetStrictFileOriginPolicy()
     {
         return sStrictFileOriginPolicy;
     }
--- a/caps/src/nsPrincipal.cpp
+++ b/caps/src/nsPrincipal.cpp
@@ -240,18 +240,41 @@ nsPrincipal::EqualsConsideringDomain(nsI
     return NS_OK;
   }
 
   if (aOther == this) {
     *aResult = true;
     return NS_OK;
   }
 
-  *aResult = NS_SUCCEEDED(
-               nsScriptSecurityManager::CheckSameOriginPrincipal(this, aOther));
+  if (!nsScriptSecurityManager::AppAttributesEqual(this, aOther)) {
+      return NS_OK;
+  }
+
+  // If either the subject or the object has changed its principal by
+  // explicitly setting document.domain then the other must also have
+  // done so in order to be considered the same origin. This prevents
+  // DNS spoofing based on document.domain (154930)
+
+  nsCOMPtr<nsIURI> thisURI;
+  this->GetDomain(getter_AddRefs(thisURI));
+  bool thisSetDomain = !!thisURI;
+  if (!thisURI) {
+      this->GetURI(getter_AddRefs(thisURI));
+  }
+
+  nsCOMPtr<nsIURI> otherURI;
+  aOther->GetDomain(getter_AddRefs(otherURI));
+  bool otherSetDomain = !!otherURI;
+  if (!otherURI) {
+      aOther->GetURI(getter_AddRefs(otherURI));
+  }
+
+  *aResult = thisSetDomain == otherSetDomain &&
+             nsScriptSecurityManager::SecurityCompareURIs(thisURI, otherURI);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPrincipal::Equals(nsIPrincipal *aOther, bool *aResult)
 {
   *aResult = false;
 
--- a/caps/src/nsScriptSecurityManager.cpp
+++ b/caps/src/nsScriptSecurityManager.cpp
@@ -481,81 +481,16 @@ nsScriptSecurityManager::CheckSameOrigin
             ReportError(nullptr, NS_LITERAL_STRING("CheckSameOriginError"),
                      aSourceURI, aTargetURI);
          }
          return NS_ERROR_DOM_BAD_URI;
     }
     return NS_OK;
 }
 
-/* static */
-nsresult
-nsScriptSecurityManager::CheckSameOriginPrincipal(nsIPrincipal* aSubject,
-                                                  nsIPrincipal* aObject)
-{
-    /*
-    ** Get origin of subject and object and compare.
-    */
-    if (aSubject == aObject)
-        return NS_OK;
-
-    if (!AppAttributesEqual(aSubject, aObject)) {
-        return NS_ERROR_DOM_PROP_ACCESS_DENIED;
-    }
-
-    // Default to false, and change if that turns out wrong.
-    bool subjectSetDomain = false;
-    bool objectSetDomain = false;
-    
-    nsCOMPtr<nsIURI> subjectURI;
-    nsCOMPtr<nsIURI> objectURI;
-
-    aSubject->GetDomain(getter_AddRefs(subjectURI));
-    if (!subjectURI) {
-        aSubject->GetURI(getter_AddRefs(subjectURI));
-    } else {
-        subjectSetDomain = true;
-    }
-
-    aObject->GetDomain(getter_AddRefs(objectURI));
-    if (!objectURI) {
-        aObject->GetURI(getter_AddRefs(objectURI));
-    } else {
-        objectSetDomain = true;
-    }
-
-    if (SecurityCompareURIs(subjectURI, objectURI))
-    {   // If either the subject or the object has changed its principal by
-        // explicitly setting document.domain then the other must also have
-        // done so in order to be considered the same origin. This prevents
-        // DNS spoofing based on document.domain (154930)
-
-        // If both or neither explicitly set their domain, allow the access
-        if (subjectSetDomain == objectSetDomain)
-            return NS_OK;
-    }
-
-    /*
-    ** Access tests failed, so now report error.
-    */
-    return NS_ERROR_DOM_PROP_ACCESS_DENIED;
-}
-
-// It's important that
-//
-//   CheckSameOriginPrincipal(A, B) == NS_OK
-//
-// imply
-//
-//   HashPrincipalByOrigin(A) == HashPrincipalByOrigin(B)
-//
-// if principals A and B could ever be used as keys in a hashtable.
-// Violation of this invariant leads to spurious failures of hashtable
-// lookups.  See bug 454850.
-
 /*static*/ uint32_t
 nsScriptSecurityManager::HashPrincipalByOrigin(nsIPrincipal* aPrincipal)
 {
     nsCOMPtr<nsIURI> uri;
     aPrincipal->GetDomain(getter_AddRefs(uri));
     if (!uri)
         aPrincipal->GetURI(getter_AddRefs(uri));
     return SecurityHashURI(uri);