Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6613)

Unified Diff: chrome/renderer/extensions/extension_resource_request_policy.cc

Issue 10828067: Extension resources should only load in contexts the extension has permission to access. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Line endings. How I hate them. Created 8 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..091538c17b3150d3b2e2ff7240ce5744e59358a1 100644
--- a/chrome/renderer/extensions/extension_resource_request_policy.cc
+++ b/chrome/renderer/extensions/extension_resource_request_policy.cc
@@ -7,6 +7,7 @@
#include "base/command_line.h"
#include "base/logging.h"
#include "base/stringprintf.h"
+#include "base/string_util.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/url_constants.h"
#include "chrome/common/extensions/extension.h"
@@ -47,27 +48,44 @@ 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();
+
+ // 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/139788.
+ if (frame_url == GURL(chrome::kAboutBlankURL) && frame->parent())
+ frame_url = frame->parent()->document().url();
+
+ bool extension_has_access_to_frame =
+ extension->GetEffectiveHostPermissions().MatchesURL(frame_url);
+ bool frame_has_empty_origin = frame_url.is_empty();
+ bool frame_is_data_url = frame_url.SchemeIs(chrome::kDataScheme);
+ bool frame_is_devtools = frame_url.SchemeIs(chrome::kChromeDevToolsScheme) &&
+ !extension->devtools_url().is_empty();
+ bool frame_is_extension = frame_url.SchemeIs(chrome::kExtensionScheme);
+ bool is_own_resource = frame_url.GetOrigin() == extension->url();
+ bool is_resource_nacl_manifest =
+ extension->IsResourceNaClManifest(resource_url.path());
+ bool is_resource_web_accessible =
+ extension->IsResourceWebAccessible(resource_url.path()) ||
+ CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kDisableExtensionsResourceWhitelist);
+
+ // Given that the goal here is to prevent malicious injection of a benign
+ // extension's content into a context where it might be damaging, allowing
+ // unvalidated "nexe" resources is low-risk. If a mechanism for synchronously
+ // validating that the "nexe" is a NaCl executable appears, we should use it.
+ bool is_resource_nexe = extension->HasNaClModules() &&
+ EndsWith(resource_url.path(), ".nexe", true);
+
+ if (!frame_has_empty_origin && !frame_is_devtools && !is_own_resource) {
+ 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 +96,18 @@ bool ExtensionResourceRequestPolicy::CanRequestResource(
WebKit::WebString::fromUTF8(message)));
return false;
}
+
+ if (!extension_has_access_to_frame && !frame_is_extension &&
+ !frame_is_data_url && !is_resource_nacl_manifest && !is_resource_nexe) {
+ 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;

Powered by Google App Engine
This is Rietveld 408576698