Extension resources should only load in contexts the extension has permission to access.

chrome/renderer/extensions/extension_resource_request_policy.cc

Index: chrome/renderer/extensions/extension_resource_request_policy.cc
diff --git a/chrome/renderer/extensions/extension_resource_request_policy.cc b/chrome/renderer/extensions/extension_resource_request_policy.cc
index 9f1e8428b3fdd523a22297231fcbad0c2d721a9a..5a631a6b5c6f73f067d4fa9fcf4346546ba20652 100644
--- a/chrome/renderer/extensions/extension_resource_request_policy.cc
+++ b/chrome/renderer/extensions/extension_resource_request_policy.cc
@@ -47,27 +47,55 @@ bool ExtensionResourceRequestPolicy::CanRequestResource(
     return false;
   }
 
-  // Disallow loading of extension resources which are not explicitely listed
-  // as web accessible if the manifest version is 2 or greater.
-  if (!extension->IsResourceWebAccessible(resource_url.path()) &&
-      !CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableExtensionsResourceWhitelist)) {
-    GURL frame_url = frame->document().url();
-    GURL page_url = frame->top()->document().url();
-
-    // Exceptions are:
-    // - empty origin (needed for some edge cases when we have empty origins)
-    bool is_empty_origin = frame_url.is_empty();
-    // - extensions requesting their own resources (frame_url check is for
-    //     images, page_url check is for iframes)
-    bool is_own_resource = frame_url.GetOrigin() == extension->url() ||
-        page_url.GetOrigin() == extension->url();
-    // - devtools (chrome-extension:// URLs are loaded into frames of devtools
-    //     to support the devtools extension APIs)
-    bool is_dev_tools = page_url.SchemeIs(chrome::kChromeDevToolsScheme) &&
-        !extension->devtools_url().is_empty();
-
-    if (!is_empty_origin && !is_own_resource && !is_dev_tools) {
+  GURL frame_url = frame->document().url();
+
+  // Loading a resource into an empty iframe should check against the iframe's

[Aaron Boodman, 2012/07/31 09:50:10]

How about:

// In the case of loading a frame, frame* points to the frame 
// being loaded, not the containing frame. This means that
// frame->document().url() ends up not being useful to us.
//
// WebKit doesn't currently pass us enough information to
// know when we're a frame, so we hack it by checking for
// 'about:blank', which should only happen in this 
// situation.
//
// TODO(aa): Fix WebKit to pass the context of the load.
// crbug.com/xxxx

+  // parent's URL, not `about:blank`. We don't have a good mechanism to
+  // determine if we're actually in an iframe, so checking the parent whenever
+  // we're loading into an `about:blank` frame_url is the best we can do.
+  if (frame_url == GURL(chrome::kAboutBlankURL) && frame->parent())
+      frame_url = frame->parent()->document().url();
+
+  // Disallow loading of extension resources when one of the following
+  // conditions holds:
+  //
+  // 1.  The resource is not explicitly listed as a web accessible resource (and
+  //     this check isn't disabled via a command-line flag).
+  bool is_resource_web_accessible =
+      extension->IsResourceWebAccessible(resource_url.path()) ||
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableExtensionsResourceWhitelist);
+
+  // 2.  The resource is loaded into a context for which the extension has no
+  //     permission (e.g. resources from an extension with host permissions for
+  //     `evil.com` shouldn't be loaded into `example.com`).
+  bool is_access_permitted =

[Aaron Boodman, 2012/07/31 09:50:10]

is_access_permitted is too general to be meaningful in this function (the whole
function is about access being permitted).

How about extension_has_access_to_frame.

+      extension->GetEffectiveHostPermissions().MatchesURL(frame_url);
+
+  // Exceptions are made for the following cases for both of the above:
+  //
+  // 1.  Empty origins (needed for some edge cases when we have empty origins).
+  bool is_empty_origin = frame_url.is_empty();
+
+  // 2.  Extensions requesting their own resources.
+  bool is_own_resource = frame_url.GetOrigin() == extension->url();
+
+  // 3.  Devtools (chrome-extension:// URLs are loaded into frames of devtools
+  //     to support the devtools extension APIs).
+  bool is_dev_tools = frame_url.SchemeIs(chrome::kChromeDevToolsScheme) &&
+      !extension->devtools_url().is_empty();
+
+  // Exceptions are made to the host permission restriction for the following
+  // cases.
+  //
+  // 4.  `data:` origins.
+  bool is_data_origin = frame_url.SchemeIs(chrome::kDataScheme);
+
+  // 5.  `chrome-extension:` origins.
+  bool is_extension_origin = frame_url.SchemeIs(chrome::kExtensionScheme);
+
+  if (!is_empty_origin && !is_own_resource && !is_dev_tools) {

[Aaron Boodman, 2012/07/31 09:50:10]

I think the commentary makes this harder to read. With some variable renaming, I
think it can stand with far fewer comments.

bool is_resource_web_accessible = ...
bool extension_has_access_to_frame = ...
bool frame_has_empty_origin = ...
bool is_own_resource = ...
bool frame_is_devtools = ...
bool frame_is_data_url = ...
bool frame_is_extension = ...

if (!frame_has_empty_origin && !is_own_resource && !frame_is_devtools) {
  if (!is_resource_web_accessible) {
    // ... error message
    return false;
  }

  if (!extension_has_access_to_frame && !frame_is_extension &&
!frame_is_data_url) {
    // ... error
    return false;
  }
}

+    if (!is_resource_web_accessible) {
       std::string message = base::StringPrintf(
           "Denying load of %s. Resources must be listed in the "
           "web_accessible_resources manifest key in order to be loaded by "
@@ -78,6 +106,17 @@ bool ExtensionResourceRequestPolicy::CanRequestResource(
                                     WebKit::WebString::fromUTF8(message)));
       return false;
     }
+
+    if (!is_access_permitted && !is_extension_origin && !is_data_origin) {
+      std::string message = base::StringPrintf(
+          "Denying load of %s. An extension's resources can only be loaded "
+          "into a page for which the extension has explicit host permissions.",
+          resource_url.spec().c_str());
+      frame->addMessageToConsole(
+          WebKit::WebConsoleMessage(WebKit::WebConsoleMessage::LevelError,
+                                    WebKit::WebString::fromUTF8(message)));
+      return false;
+    }
   }
 
   return true;