Skip to content

GitLab

T
tangled
Commit e8d9c420 authored by nkostylev@chromium.org's avatar nkostylev@chromium.org
Browse files
Options

Revert 149610 - Extension resources should only load in contexts the extension... 

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

See http://codereview.chromium.org/10792008/ for background.

BUG=139592

Review URL: https://chromiumcodereview.appspot.com/10828067

TBR=mkwst@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10826134

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149760 0039d316-1c4b-4281-b951-d872f2087c98
parent dd5287ab
Hide whitespace changes
Inline Side-by-side
Showing with 33 additions and 97 deletions
chrome/browser/extensions/extension_resource_request_policy_apitest.cc
View file @ e8d9c420
......@@ -54,15 +54,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionResourceRequestPolicyTest, OriginPrivileges) {
&result));
EXPECT_EQ(result, "Loaded");
// A web host that does not have permission.
ui_test_utils::NavigateToURL(
browser(), web_resource.ReplaceComponents(make_host_b_com));
ASSERT_TRUE(content::ExecuteJavaScriptAndExtractString(
chrome::GetActiveWebContents(browser())->GetRenderViewHost(), L"",
L"window.domAutomationController.send(document.title)",
&result));
EXPECT_EQ(result, "Image failed to load");
// A web host that loads a non-existent extension.
GURL non_existent_extension(
test_server()->GetURL(
......
chrome/common/extensions/extension.cc
View file @ e8d9c420
......@@ -536,27 +536,10 @@ bool Extension::HasWebAccessibleResources() const {
return false;
}
bool Extension::HasNaClModules() const {
if (nacl_modules_.size())
return true;
return false;
}
bool Extension::IsSandboxedPage(const std::string& relative_path) const {
return ResourceMatches(sandboxed_pages_, relative_path);
}
bool Extension::IsResourceNaClManifest(const std::string& resource) const {
GURL url = extension_url_.Resolve(resource);
for (std::vector<NaClModuleInfo>::const_iterator it = nacl_modules_.begin();
it != nacl_modules_.end(); it++) {
if (it->url == url)
return true;
}
return false;
}
std::string Extension::GetResourceContentSecurityPolicy(
const std::string& relative_path) const {
......
chrome/common/extensions/extension.h
View file @ e8d9c420
......@@ -356,9 +356,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
// Returns true if the specified resource is web accessible.
bool IsResourceWebAccessible(const std::string& relative_path) const;
// Returns true if the specified resource is a NaCl manifest.
bool IsResourceNaClManifest(const std::string& resource) const;
// Returns true if the specified page is sandboxed (served in a unique
// origin).
bool IsSandboxedPage(const std::string& relative_path) const;
......@@ -371,9 +368,6 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
// Returns true when 'web_accessible_resources' are defined for the extension.
bool HasWebAccessibleResources() const;
// Returns true when 'nacl_modules' are defined for the extension.
bool HasNaClModules() const;
// Returns an extension resource object. |relative_path| should be UTF8
// encoded.
ExtensionResource GetResource(const std::string& relative_path) const;
......
chrome/renderer/extensions/extension_resource_request_policy.cc
View file @ e8d9c420
......@@ -7,7 +7,6 @@
#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"
......@@ -48,44 +47,27 @@ bool ExtensionResourceRequestPolicy::CanRequestResource(
return false;
}
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) {
// 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) {
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 "
......@@ -96,18 +78,6 @@ 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;
......
chrome/test/data/extensions/api_test/extension_resource_request_policy/extension/manifest.json
View file @ e8d9c420
......@@ -2,6 +2,6 @@
"description": "Extension 1",
"key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDPOziAf8MbTjdUo6DysZ4nAU/2f/kwYnftyKkxI1GyTlbStprGy+Y2ek4/59QbE3xEE+dIIuYeObM4QTptpcFMg956ZLFoeDg41Pg3tzUrbltgG8hXTbBxN852FJx2kdaqa/MKUUsJKGSD5hkUmvZRADGGWhMWzvz64ao1h02xJQIDAQAB",
"name": "test",
"permissions": [ "http://a.com/*", "http://127.0.0.1/*" ],
"permissions": [ "http://a.com/*" ],
"version": "0.1"
}
chrome/test/data/extensions/api_test/extension_resource_request_policy/extension2/manifest.json
View file @ e8d9c420
......@@ -2,6 +2,5 @@
"description": "Extension 2",
"key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDPOziAf8MbTjdUo6DysZ4nAU/2f/kwYnftyKkxI1GyTlbStprGy+Y2ek4/59QbE3xEE+dIIuYeObM4QTptpcFMg956ZLFoeDg41Pg3tzUrbltgG8hXTbBxN852FJx2kdaqa/MKUUsJKGSD5hkUmvZRADGGWhMWzvz64ao1h02xJQIDAQAC",
"name": "test",
"version": "0.1",
"permissions": [ "http://a.com/*", "http://127.0.0.1/*" ]
"version": "0.1"
}
chrome/test/data/extensions/api_test/extension_resource_request_policy/web_accessible/manifest.json
View file @ e8d9c420
{
"manifest_version": 2,
"name": "web_accessible",
"version": "0.1",
"permissions": [ "http://a.com/*", "http://127.0.0.1/*" ],
"key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDPOziAf8MbTjdUo6DysZ4nAU/2f/kwYnftyKkxI1GyTlbStprGy+Y2ek4/59QbE3xEE+dIIuYeObM4QTptpcFMg956ZLFoeDg41Pg3tzUrbltgG8hXTbBxN852FJx2kdaqa/MKUUsJKGSD5hkUmvZRADGGWhMWzvz64ao1h02xJQIDAQAD",
"web_accessible_resources": [
"test.png",
"nonexistent.png"
]
}
{
"manifest_version": 2,
"name": "web_accessible",
"version": "0.1",
"key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDPOziAf8MbTjdUo6DysZ4nAU/2f/kwYnftyKkxI1GyTlbStprGy+Y2ek4/59QbE3xEE+dIIuYeObM4QTptpcFMg956ZLFoeDg41Pg3tzUrbltgG8hXTbBxN852FJx2kdaqa/MKUUsJKGSD5hkUmvZRADGGWhMWzvz64ao1h02xJQIDAQAD",
"web_accessible_resources": [
"test.png",
"nonexistent.png"
]
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment