Bug 1335272 - fix about:cache internal links, r?bz draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 02 Feb 2017 15:10:11 +0000
changeset 480054 5498ab3e5d86f7025ab4e3e63e0dafd0ac964b74
parent 480053 0edc732e50ea3482d0cba0fd150ac4d0a07aaa82
child 480055 c200cbf9c05548363af2f47bb91f95e73800783f
child 480056 58dc8622d703fb468049e774716737bebdd04828
push id44444
push usergijskruitbosch@gmail.com
push dateTue, 07 Feb 2017 19:46:28 +0000
reviewersbz
bugs1335272
milestone54.0a1
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)