Bug 1335272 - fix about:cache internal links, r=bz
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 02 Feb 2017 15:10:11 +0000
changeset 390212 494c40293b333e818e795482aad8a6c713a1cd48
parent 390211 38740bd3a50e9d9ecec61bedc69092c1f49def88
child 390213 54f31b2c699eb3e83ca6a8896727b078543548b2
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1335272
milestone54.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 1335272 - fix about:cache internal links, r=bz MozReview-Commit-ID: QzgsTTulJC
caps/nsScriptSecurityManager.cpp
caps/tests/mochitest/browser_checkloaduri.js
netwerk/protocol/about/nsAboutCache.cpp
netwerk/protocol/about/nsAboutCacheEntry.cpp
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -727,24 +727,58 @@ nsScriptSecurityManager::CheckLoadURIWit
     if (NS_FAILED(rv)) return rv;
 
     while (currentURI && currentOtherURI) {
         nsAutoCString scheme, otherScheme;
         currentURI->GetScheme(scheme);
         currentOtherURI->GetScheme(otherScheme);
 
         bool schemesMatch = scheme.Equals(otherScheme, stringComparator);
-        bool isSamePage;
+        bool isSamePage = false;
         // about: URIs are special snowflakes.
-        if (scheme.EqualsLiteral("about")) {
-            nsAutoCString module, otherModule;
-            isSamePage = schemesMatch &&
-                NS_SUCCEEDED(NS_GetAboutModuleName(currentURI, module)) &&
-                NS_SUCCEEDED(NS_GetAboutModuleName(currentOtherURI, otherModule)) &&
-                module.Equals(otherModule);
+        if (scheme.EqualsLiteral("about") && schemesMatch) {
+            nsAutoCString moduleName, otherModuleName;
+            // about: pages can always link to themselves:
+            isSamePage =
+              NS_SUCCEEDED(NS_GetAboutModuleName(currentURI, moduleName)) &&
+              NS_SUCCEEDED(NS_GetAboutModuleName(currentOtherURI, otherModuleName)) &&
+              moduleName.Equals(otherModuleName);
+            if (!isSamePage) {
+                // We will have allowed the load earlier if the source page has
+                // system principal. So we know the source has a content
+                // principal, and it's trying to link to something else.
+                // Linkable about: pages are always reachable, even if we hit
+                // the CheckLoadURIFlags call below.
+                // We punch only 1 other hole: iff the source is unlinkable,
+                // we let them link to other pages explicitly marked SAFE
+                // for content. This avoids world-linkable about: pages linking
+                // to non-world-linkable about: pages.
+                nsCOMPtr<nsIAboutModule> module, otherModule;
+                bool knowBothModules =
+                    NS_SUCCEEDED(NS_GetAboutModule(currentURI, getter_AddRefs(module))) &&
+                    NS_SUCCEEDED(NS_GetAboutModule(currentOtherURI, getter_AddRefs(otherModule)));
+                uint32_t aboutModuleFlags = 0;
+                uint32_t otherAboutModuleFlags = 0;
+                knowBothModules = knowBothModules &&
+                    NS_SUCCEEDED(module->GetURIFlags(currentURI, &aboutModuleFlags)) &&
+                    NS_SUCCEEDED(otherModule->GetURIFlags(currentOtherURI, &otherAboutModuleFlags));
+                if (knowBothModules) {
+                    isSamePage =
+                        !(aboutModuleFlags & nsIAboutModule::MAKE_LINKABLE) &&
+                        (otherAboutModuleFlags & nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT);
+                    if (isSamePage && otherAboutModuleFlags & nsIAboutModule::MAKE_LINKABLE) {
+                        //XXXgijs: this is a hack. The target will be nested
+                        // (with innerURI of moz-safe-about:whatever), and
+                        // the source isn't, so we won't pass if we finish
+                        // the loop. We *should* pass, though, so return here.
+                        // This hack can go away when bug 1228118 is fixed.
+                        return NS_OK;
+                    }
+                }
+            }
         } else {
             bool equalExceptRef = false;
             rv = currentURI->EqualsExceptRef(currentOtherURI, &equalExceptRef);
             isSamePage = NS_SUCCEEDED(rv) && equalExceptRef;
         }
 
         // If schemes are not equal, or they're equal but the target URI
         // is different from the source URI and doesn't always allow linking
--- a/caps/tests/mochitest/browser_checkloaduri.js
+++ b/caps/tests/mochitest/browser_checkloaduri.js
@@ -1,11 +1,48 @@
 "use strict";
 
 let ssm = Services.scriptSecurityManager;
+// This will show a directory listing, but we never actually load these so that's OK.
+const kDummyPage = getRootDirectory(gTestPath);
+
+const kAboutPagesRegistered = Promise.all([
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-chrome-privs", kDummyPage,
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-chrome-privs2", kDummyPage,
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-unknown-linkable", kDummyPage,
+    Ci.nsIAboutModule.MAKE_LINKABLE | Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-unknown-linkable2", kDummyPage,
+    Ci.nsIAboutModule.MAKE_LINKABLE | Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-unknown-unlinkable", kDummyPage,
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-unknown-unlinkable2", kDummyPage,
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-content-unlinkable", kDummyPage,
+    Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT | Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-content-unlinkable2", kDummyPage,
+    Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT | Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-content-linkable", kDummyPage,
+    Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT | Ci.nsIAboutModule.MAKE_LINKABLE |
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+  BrowserTestUtils.registerAboutPage(
+    registerCleanupFunction, "test-content-linkable2", kDummyPage,
+    Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT | Ci.nsIAboutModule.MAKE_LINKABLE |
+    Ci.nsIAboutModule.ALLOW_SCRIPT),
+]);
 
 const URLs = new Map([
   ["http://www.example.com", [
   // For each of these entries, the booleans represent whether the parent URI can:
   // - load them
   // - load them without principal inheritance
   // - whether the URI can be created at all (some protocol handlers will
   //   refuse to create certain variants)
@@ -17,65 +54,171 @@ const URLs = new Map([
     ["view-source:http://www.example2.com", false, false, true],
     ["view-source:https://www.example2.com", false, false, true],
     ["view-source:feed:http://www.example2.com", false, false, true],
     ["feed:view-source:http://www.example2.com", false, false, false],
     ["data:text/html,Hi", true, false, true],
     ["view-source:data:text/html,Hi", false, false, true],
     ["javascript:alert('hi')", true, false, true],
     ["moz://a", false, false, true],
+    ["about:test-chrome-privs", false, false, true],
+    ["about:test-unknown-unlinkable", false, false, true],
+    ["about:test-content-unlinkable", false, false, true],
+    ["about:test-content-linkable", true, true, true],
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable", false, false, true],
   ]],
   ["feed:http://www.example.com", [
     ["http://www.example2.com", true, true, true],
     ["feed:http://www.example2.com", true, true, true],
     ["https://www.example2.com", true, true, true],
     ["feed:https://www.example2.com", true, true, true],
     ["chrome://foo/content/bar.xul", false, false, true],
     ["feed:chrome://foo/content/bar.xul", false, false, false],
     ["view-source:http://www.example2.com", false, false, true],
     ["view-source:https://www.example2.com", false, false, true],
     ["view-source:feed:http://www.example2.com", false, false, true],
     ["feed:view-source:http://www.example2.com", false, false, false],
     ["data:text/html,Hi", true, false, true],
     ["view-source:data:text/html,Hi", false, false, true],
     ["javascript:alert('hi')", true, false, true],
     ["moz://a", false, false, true],
+    ["about:test-chrome-privs", false, false, true],
+    ["about:test-unknown-unlinkable", false, false, true],
+    ["about:test-content-unlinkable", false, false, true],
+    ["about:test-content-linkable", true, true, true],
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable", false, false, true],
   ]],
   ["view-source:http://www.example.com", [
     ["http://www.example2.com", true, true, true],
     ["feed:http://www.example2.com", false, false, true],
     ["https://www.example2.com", true, true, true],
     ["feed:https://www.example2.com", false, false, true],
     ["chrome://foo/content/bar.xul", false, false, true],
     ["feed:chrome://foo/content/bar.xul", false, false, false],
     ["view-source:http://www.example2.com", true, true, true],
     ["view-source:https://www.example2.com", true, true, true],
     ["view-source:feed:http://www.example2.com", false, false, true],
     ["feed:view-source:http://www.example2.com", false, false, false],
     ["data:text/html,Hi", true, false, true],
     ["view-source:data:text/html,Hi", true, false, true],
     ["javascript:alert('hi')", true, false, true],
     ["moz://a", false, false, true],
+    ["about:test-chrome-privs", false, false, true],
+    ["about:test-unknown-unlinkable", false, false, true],
+    ["about:test-content-unlinkable", false, false, true],
+    ["about:test-content-linkable", true, true, true],
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable", false, false, true],
   ]],
-  ["about:foo", [
-    ["about:foo?", true, true, true],
-    ["about:foo?bar", true, true, true],
-    ["about:foo#", true, true, true],
-    ["about:foo#bar", true, true, true],
-    ["about:foo?#", true, true, true],
-    ["about:foo?bar#baz", true, true, true],
-    ["about:bar", false, false, true],
-    ["about:bar?foo#baz", false, false, true],
-    ["about:bar?foo", false, false, true],
-    ["http://www.example.com/", true, true, true],
-    ["moz://a", false, false, true],
+  // about: related tests.
+  ["about:test-chrome-privs", [
+    ["about:test-chrome-privs", true, true, true],
+    ["about:test-chrome-privs2", true, true, true],
+    ["about:test-chrome-privs2?foo#bar", true, true, true],
+    ["about:test-chrome-privs2?foo", true, true, true],
+    ["about:test-chrome-privs2#bar", true, true, true],
+
+    ["about:test-unknown-unlinkable", true, true, true],
+
+    ["about:test-content-unlinkable", true, true, true],
+    ["about:test-content-unlinkable?foo", true, true, true],
+    ["about:test-content-unlinkable?foo#bar", true, true, true],
+    ["about:test-content-unlinkable#bar", true, true, true],
+
+    ["about:test-content-linkable", true, true, true],
+
+    ["about:test-unknown-linkable", true, true, true],
+  ]],
+  ["about:test-unknown-unlinkable", [
+    ["about:test-chrome-privs", false, false, true],
+
+    // Can link to ourselves:
+    ["about:test-unknown-unlinkable", true, true, true],
+    // Can't link to unlinkable content if we're not sure it's privileged:
+    ["about:test-unknown-unlinkable2", false, false, true],
+
+    ["about:test-content-unlinkable", true, true, true],
+    ["about:test-content-unlinkable2", true, true, true],
+    ["about:test-content-unlinkable2?foo", true, true, true],
+    ["about:test-content-unlinkable2?foo#bar", true, true, true],
+    ["about:test-content-unlinkable2#bar", true, true, true],
+
+    ["about:test-content-linkable", true, true, true],
+
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable", false, false, true],
+  ]],
+  ["about:test-content-unlinkable", [
+    ["about:test-chrome-privs", false, false, true],
+
+    // Can't link to unlinkable content if we're not sure it's privileged:
+    ["about:test-unknown-unlinkable", false, false, true],
+
+    ["about:test-content-unlinkable", true, true, true],
+    ["about:test-content-unlinkable2", true, true, true],
+    ["about:test-content-unlinkable2?foo", true, true, true],
+    ["about:test-content-unlinkable2?foo#bar", true, true, true],
+    ["about:test-content-unlinkable2#bar", true, true, true],
+
+    ["about:test-content-linkable", true, true, true],
+    ["about:test-unknown-linkable", false, false, true],
+  ]],
+  ["about:test-unknown-linkable", [
+    ["about:test-chrome-privs", false, false, true],
+
+    // Linkable content can't link to unlinkable content.
+    ["about:test-unknown-unlinkable", false, false, true],
+
+    ["about:test-content-unlinkable", false, false, true],
+    ["about:test-content-unlinkable2", false, false, true],
+    ["about:test-content-unlinkable2?foo", false, false, true],
+    ["about:test-content-unlinkable2?foo#bar", false, false, true],
+    ["about:test-content-unlinkable2#bar", false, false, true],
+
+    // ... but it can link to other linkable content.
+    ["about:test-content-linkable", true, true, true],
+
+    // Can link to ourselves:
+    ["about:test-unknown-linkable", true, true, true],
+
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable2", false, false, true],
+  ]],
+  ["about:test-content-linkable", [
+    ["about:test-chrome-privs", false, false, true],
+
+    // Linkable content can't link to unlinkable content.
+    ["about:test-unknown-unlinkable", false, false, true],
+
+    ["about:test-content-unlinkable", false, false, true],
+
+    // ... but it can link to itself and other linkable content.
+    ["about:test-content-linkable", true, true, true],
+    ["about:test-content-linkable2", true, true, true],
+
+    // Because this page doesn't have SAFE_FOR_UNTRUSTED, the web can't link to it:
+    ["about:test-unknown-linkable", false, false, true],
   ]],
 ]);
 
 function testURL(source, target, canLoad, canLoadWithoutInherit, canCreate, flags) {
+  function getPrincipalDesc(principal) {
+    if (principal.URI) {
+      return principal.URI.spec;
+    }
+    if (principal.isSystemPrincipal) {
+      return "system principal";
+    }
+    if (principal.isNullPrincipal) {
+      return "null principal";
+    }
+    return "unknown principal";
+  }
   let threw = false;
   let targetURI;
   try {
     targetURI = makeURI(target);
   } catch (ex) {
     ok(!canCreate, "Shouldn't be passing URIs that we can't create. Failed to create: " + target);
     return;
   }
@@ -86,24 +229,30 @@ function testURL(source, target, canLoad
   } catch (ex) {
     info(ex.message);
     threw = true;
   }
   let inheritDisallowed = flags & ssm.DISALLOW_INHERIT_PRINCIPAL;
   let shouldThrow = inheritDisallowed ? !canLoadWithoutInherit : !canLoad;
   ok(threw == shouldThrow,
      "Should " + (shouldThrow ? "" : "not ") + "throw an error when loading " +
-     target + " from " + source.URI.spec +
+     target + " from " + getPrincipalDesc(source) +
      (inheritDisallowed ? " without" : " with") + " principal inheritance.");
 }
 
 add_task(function* () {
+  yield kAboutPagesRegistered;
   let baseFlags = ssm.STANDARD | ssm.DONT_REPORT_ERRORS;
   for (let [sourceString, targetsAndExpectations] of URLs) {
-    let source = ssm.createCodebasePrincipal(makeURI(sourceString), {});
+    let source;
+    if (sourceString.startsWith("about:test-chrome-privs")) {
+      source = ssm.getSystemPrincipal();
+    } else {
+      source = ssm.createCodebasePrincipal(makeURI(sourceString), {});
+    }
     for (let [target, canLoad, canLoadWithoutInherit, canCreate] of targetsAndExpectations) {
       testURL(source, target, canLoad, canLoadWithoutInherit, canCreate, baseFlags);
       testURL(source, target, canLoad, canLoadWithoutInherit, canCreate,
               baseFlags | ssm.DISALLOW_INHERIT_PRINCIPAL);
     }
   }
 
   // Now test blob URIs, which we need to do in-content.
--- a/netwerk/protocol/about/nsAboutCache.cpp
+++ b/netwerk/protocol/about/nsAboutCache.cpp
@@ -560,17 +560,17 @@ nsAboutCache::Channel::FlushBuffer()
     }
 
     return rv;
 }
 
 NS_IMETHODIMP
 nsAboutCache::GetURIFlags(nsIURI *aURI, uint32_t *result)
 {
-    *result = 0;
+    *result = nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT;
     return NS_OK;
 }
 
 // static
 nsresult
 nsAboutCache::Create(nsISupports *aOuter, REFNSIID aIID, void **aResult)
 {
     nsAboutCache* about = new nsAboutCache();
--- a/netwerk/protocol/about/nsAboutCacheEntry.cpp
+++ b/netwerk/protocol/about/nsAboutCacheEntry.cpp
@@ -107,17 +107,18 @@ nsAboutCacheEntry::NewChannel(nsIURI* ur
     channel.forget(result);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAboutCacheEntry::GetURIFlags(nsIURI *aURI, uint32_t *result)
 {
-    *result = nsIAboutModule::HIDE_FROM_ABOUTABOUT;
+    *result = nsIAboutModule::HIDE_FROM_ABOUTABOUT |
+              nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT;
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsAboutCacheEntry::Channel
 
 nsresult
 nsAboutCacheEntry::Channel::Init(nsIURI* uri, nsILoadInfo* aLoadInfo)