Bug 816956 - Don't try to check all parent hosts in the Permission Manager (but only until the domain name). r=sicking

extensions/cookie/nsPermissionManager.cpp

@@ -29,6 +29,7 @@
 #include "nsIScriptSecurityManager.h"
 #include "nsIAppsService.h"
 #include "mozIApplication.h"
+#include "nsIEffectiveTLDService.h"
 
 static nsPermissionManager *gPermissionManager = nullptr;
 
@@ -126,6 +127,27 @@ GetHostForPrincipal(nsIPrincipal* aPrincipal, nsACString& aHost)
   return NS_OK;
 }
 
+nsCString
+GetNextSubDomainForHost(const nsACString& aHost)
+{
+  nsCOMPtr<nsIEffectiveTLDService> tldService =
+    do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
+  if (!tldService) {
+    NS_ERROR("Should have a tld service!");
+    return EmptyCString();
+  }
+
+  nsCString subDomain;
+  nsresult rv = tldService->GetNextSubDomain(aHost, subDomain);
+  // We can fail if there is no more subdomain or if the host can't have a
+  // subdomain.
+  if (NS_FAILED(rv)) {
+    return EmptyCString();
+  }
+
+  return subDomain;
+}
+
 class AppClearDataObserver MOZ_FINAL : public nsIObserver {
 public:
   NS_DECL_ISUPPORTS
@@ -1059,41 +1081,36 @@ nsPermissionManager::GetPermissionHashKey(const nsACString& aHost,
                                           uint32_t aType,
                                           bool aExactHostMatch)
 {
-  uint32_t offset = 0;
-  PermissionHashKey* entry;
-  int64_t now = PR_Now() / 1000;
+  PermissionHashKey* entry = nullptr;
 
-  do {
-    nsRefPtr<PermissionKey> key = new PermissionKey(Substring(aHost, offset), aAppId, aIsInBrowserElement);
-    entry = mPermissionTable.GetEntry(key);
+  nsRefPtr<PermissionKey> key = new PermissionKey(aHost, aAppId, aIsInBrowserElement);
+  entry = mPermissionTable.GetEntry(key);
 
-    if (entry) {
-      PermissionEntry permEntry = entry->GetPermission(aType);
+  if (entry) {
+    PermissionEntry permEntry = entry->GetPermission(aType);
 
-      // if the entry is expired, remove and keep looking for others.
-      if (permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME &&
-          permEntry.mExpireTime <= now) {
-        nsCOMPtr<nsIPrincipal> principal;
-        if (NS_FAILED(GetPrincipal(aHost, aAppId, aIsInBrowserElement, getter_AddRefs(principal)))) {
-          return nullptr;
-        }
-
-        RemoveFromPrincipal(principal, mTypeArray[aType].get());
-      } else if (permEntry.mPermission != nsIPermissionManager::UNKNOWN_ACTION) {
-        break;
+    // if the entry is expired, remove and keep looking for others.
+    if (permEntry.mExpireType == nsIPermissionManager::EXPIRE_TIME &&
+        permEntry.mExpireTime <= (PR_Now() / 1000)) {
+      nsCOMPtr<nsIPrincipal> principal;
+      if (NS_FAILED(GetPrincipal(aHost, aAppId, aIsInBrowserElement, getter_AddRefs(principal)))) {
+        return nullptr;
       }
 
-      // reset entry, to be able to return null on failure
+      entry = nullptr;
+      RemoveFromPrincipal(principal, mTypeArray[aType].get());
+    } else if (permEntry.mPermission == nsIPermissionManager::UNKNOWN_ACTION) {
       entry = nullptr;
     }
-    if (aExactHostMatch)
-      break; // do not try super domains
+  }
 
-    offset = aHost.FindChar('.', offset) + 1;
+  if (!entry && !aExactHostMatch) {
+    nsCString domain = GetNextSubDomainForHost(aHost);
+    if (!domain.IsEmpty()) {
+      return GetPermissionHashKey(domain, aAppId, aIsInBrowserElement, aType, aExactHostMatch);
+    }
+  }
 
-  // walk up the domaintree (we stop as soon as we find a match,
-  // which will be the most specific domain we have an entry for).
-  } while (offset > 0);
   return entry;
 }
 

extensions/cookie/test/unit/test_permmanager_subdomains.js

@@ -0,0 +1,56 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+function getPrincipalFromURI(uri) {
+  return Cc["@mozilla.org/scriptsecuritymanager;1"]
+           .getService(Ci.nsIScriptSecurityManager)
+           .getNoAppCodebasePrincipal(NetUtil.newURI(uri));
+}
+
+function run_test() {
+  var pm = Cc["@mozilla.org/permissionmanager;1"].
+           getService(Ci.nsIPermissionManager);
+
+  // Adds a permission to a sub-domain. Checks if it is working.
+  let sub1Principal = getPrincipalFromURI("http://sub1.example.com");
+  pm.addFromPrincipal(sub1Principal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // A sub-sub-domain should get the permission.
+  let subsubPrincipal = getPrincipalFromURI("http://sub.sub1.example.com");
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // Another sub-domain shouldn't get the permission.
+  let sub2Principal = getPrincipalFromURI("http://sub2.example.com");
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // Remove current permissions.
+  pm.removeFromPrincipal(sub1Principal, "test/subdomains");
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // Adding the permission to the main domain. Checks if it is working.
+  let mainPrincipal = getPrincipalFromURI("http://example.com");
+  pm.addFromPrincipal(mainPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // All sub-domains should have the permission now.
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.ALLOW_ACTION);
+
+  // Remove current permissions.
+  pm.removeFromPrincipal(mainPrincipal, "test/subdomains");
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"), pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+
+  // A sanity check that the previous implementation wasn't passing...
+  let crazyPrincipal = getPrincipalFromURI("http://com");
+  pm.addFromPrincipal(crazyPrincipal, "test/subdomains", pm.ALLOW_ACTION, 0, 0);
+  do_check_eq(pm.testPermissionFromPrincipal(crazyPrincipal, "test/subdomains"),  pm.ALLOW_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(mainPrincipal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub1Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(sub2Principal, "test/subdomains"),   pm.UNKNOWN_ACTION);
+  do_check_eq(pm.testPermissionFromPrincipal(subsubPrincipal, "test/subdomains"), pm.UNKNOWN_ACTION);
+}

extensions/cookie/test/unit/xpcshell.ini

@@ -21,6 +21,7 @@ tail =
 [test_permmanager_load_invalid_entries.js]
 skip-if = debug == true
 [test_permmanager_idn.js]
+[test_permmanager_subdomains.js]
 [test_permmanager_cleardata.js]
 [test_schema_2_migration.js]
 [test_schema_3_migration.js]