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

Unified Diff: chrome/browser/extensions/extension_protocols.cc

Issue 12457042: Non-web-accessible extension URLs should not load in non-extension processes (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixing nits. Created 7 years, 9 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
« no previous file with comments | « chrome/browser/chrome_security_exploit_browsertest.cc ('k') | chrome/chrome_tests.gypi » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..049d81386304d992faea0e609686f2467c4df7cd 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 a more precise
+ // check, since the renderer can lie about which extension has made the
+ // request.
+ 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
+ // 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 a DevTools page.
+ if (extensions::ManifestURL::GetDevToolsPage(extension).is_empty())
+ 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();
« no previous file with comments | « chrome/browser/chrome_security_exploit_browsertest.cc ('k') | chrome/chrome_tests.gypi » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698