Non-web-accessible extension URLs should not load in non-extension processes

chrome/browser/extensions/extension_protocols.cc

Index: chrome/browser/extensions/extension_protocols.cc
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 55f3cc759b85f9f7d0290af439c346bb06c8cfec..7de4a4c77ee89ac8824e38fe68844c7f3cbf416a 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -26,6 +26,7 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/incognito_handler.h"
+#include "chrome/common/extensions/manifest_url_handler.h"
 #include "chrome/common/extensions/web_accessible_resources_handler.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/resource_request_info.h"
@@ -270,6 +271,7 @@ bool ExtensionCanLoadInIncognito(const ResourceRequestInfo* info,
 // first need to find a way to get CanLoadInIncognito state into the renderers.
 bool AllowExtensionResourceLoad(net::URLRequest* request,
                                 bool is_incognito,
+                                const Extension* extension,
                                 ExtensionInfoMap* extension_info_map) {
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
 
@@ -286,6 +288,63 @@ bool AllowExtensionResourceLoad(net::URLRequest* request,
     return false;
   }
 
+  // The following checks are meant to replicate similar set of checks in the
+  // renderer process, performed by ResourceRequestPolicy::CanRequestResource.
+  // These are not exactly equivalent, because we don't have the same bits of
+  // information. The two checks need to be kept in sync as much as possible, as
+  // an exploited renderer can bypass the checks in ResourceRequestPolicy.
+
+  // Check if the extension for which this request is made is indeed loaded in
+  // the process sending the request. If not, we need to explicitly check if
+  // the resource is explicitly accessible or fits in a set of exception cases.
+  // Note: This allows a case where two extensions execute in the same renderer
+  // process to request each other's resources. We can't do more precise check,

[Charlie Reis, 2013/04/02 22:17:38]

nit: do a more

[nasko, 2013/04/03 15:51:50]

Done.

+  // since the renderer can lie which extension has made the request.

[Charlie Reis, 2013/04/02 22:17:38]

nit: can lie about

[nasko, 2013/04/03 15:51:50]

Done.

+  if (extension_info_map->process_map().Contains(
+      request->url().host(), info->GetChildID())) {
+    return true;
+  }
+
+  if (!content::PageTransitionIsWebTriggerable(info->GetPageTransition()))
+    return false;
+
+  // The following checks require that we have an actual extension object. If we
+  // don't have it, allow the request handling to continue with the rest of the
+  // checks.
+  if (!extension)
+    return true;
+
+  // Disallow loading of packaged resources for hosted apps. We don't allow
+  // hybrid hosted/packaged apps. The one exception is access to icons, since
+  // some extensions want to be able to do things like create their own
+  // launchers.
+  std::string resource_root_relative_path =
+      request->url().path().empty() ? "" : request->url().path().substr(1);
+  if (extension->is_hosted_app() &&
+      !extensions::IconsInfo::GetIcons(extension)
+          .ContainsPath(resource_root_relative_path)) {
+    LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
+               << "hosted app.";
+    return false;
+  }
+
+  // Extensions with web_accessible_resources, allow loading by regular

[Charlie Reis, 2013/04/02 22:17:38]

nit: comma should be a colon.

[nasko, 2013/04/03 15:51:50]

Done.

+  // renderers. Since not all subresources are required to be listed in a v2
+  // manifest, we must allow all loads if there are any web accessible
+  // resources. See http://crbug.com/179127.
+  if (extension->manifest_version() < 2 ||
+      extensions::WebAccessibleResourcesInfo::HasWebAccessibleResources(
+      extension)) {
+    return true;
+  }
+
+  // If there aren't any explicitly marked web accessible resources, the
+  // load should be allowed only if it is by DevTools. A close approximation is
+  // checking if the extension contains DevTools page.

[Charlie Reis, 2013/04/02 22:17:38]

nit: contains a DevTools page.
(or is it "the DevTools page"?)

[nasko, 2013/04/03 15:51:50]

Done.

+  if (extensions::ManifestURL::GetDevToolsPage(extension).is_empty()) {

[Charlie Reis, 2013/04/02 22:17:38]

nit: no braces.

[nasko, 2013/04/03 15:51:50]

Done.

+    return false;
+  }
+
   return true;
 }
 
@@ -327,17 +386,18 @@ class ExtensionProtocolHandler
 net::URLRequestJob*
 ExtensionProtocolHandler::MaybeCreateJob(
     net::URLRequest* request, net::NetworkDelegate* network_delegate) const {
+  // chrome-extension://extension-id/resource/path.js
+  const std::string& extension_id = request->url().host();
+  const Extension* extension =
+      extension_info_map_->extensions().GetByID(extension_id);
+
   // TODO(mpcomplete): better error code.
   if (!AllowExtensionResourceLoad(
-           request, is_incognito_, extension_info_map_)) {
+           request, is_incognito_, extension, extension_info_map_)) {
     return new net::URLRequestErrorJob(
         request, network_delegate, net::ERR_ADDRESS_UNREACHABLE);
   }
 
-  // chrome-extension://extension-id/resource/path.js
-  const std::string& extension_id = request->url().host();
-  const Extension* extension =
-      extension_info_map_->extensions().GetByID(extension_id);
   base::FilePath directory_path;
   if (extension)
     directory_path = extension->path();