Index: chrome/browser/extensions/extension_protocols.cc |
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc |
index be7bab6f71eb518c4a3f9f70fb75c7e56fda0f09..95f5ed0ec474f1d4b83220ef5f6f62be451e3acd 100644 |
--- a/chrome/browser/extensions/extension_protocols.cc |
+++ b/chrome/browser/extensions/extension_protocols.cc |
@@ -24,6 +24,7 @@ |
#include "chrome/common/extensions/extension.h" |
#include "chrome/common/extensions/extension_file_util.h" |
#include "chrome/common/extensions/extension_resource.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" |
@@ -265,6 +266,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); |
@@ -281,6 +283,55 @@ bool AllowExtensionResourceLoad(net::URLRequest* request, |
return false; |
} |
+ // The following checks are meant to replicate similar set of checks in the |
Charlie Reis
2013/02/22 02:45:17
Should we add a similar warning to resource_reques
nasko
2013/02/22 17:14:35
Done.
|
+ // renderer process, performed by ResourceRequestPolicy::CanRequestResource. |
+ // These are not exact equivalent, because we don't have the same bits of |
Charlie Reis
2013/02/22 02:45:17
nit: exact -> exactly
nasko
2013/02/22 17:14:35
Done.
|
+ // 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, |
+ // since the renderer can lie which extension has made the request. |
+ if (extension_info_map->process_map().Contains( |
+ request->url().host(), info->GetChildID())) { |
+ return true; |
+ } |
+ |
+ // TODO(nasko): Based on MaybeCreateJob, extension can be null, so check here. |
+ // Should we succeed or fail here? |
Charlie Reis
2013/02/22 02:45:17
Can we decide this, rather than leaving it as a TO
nasko
2013/02/22 17:14:35
Why allow it if there is no existing extension for
Matt Perry
2013/02/22 18:56:40
It likely means the extension has been unloaded, s
|
+ 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. |
Charlie Reis
2013/02/22 02:45:17
Duplicating this code from resource_request_policy
nasko
2013/02/22 17:14:35
Done.
|
+ std::string resource_root_relative_path = |
+ request->url().path().empty() ? "" : request->url().path().substr(1); |
+ if (extension->is_hosted_app() && |
+ !extension->icons().ContainsPath(resource_root_relative_path)) { |
+ LOG(ERROR) << "Denying load of " << request->url().spec() << " from " |
+ << "hosted app."; |
+ return false; |
+ } |
+ |
+ // If the resource is not expicitly marked as web accessible, it should only |
+ // be allowed if it is being loaded by DevTools. A close approximation is |
+ // checking if the extension contains DevTools page. |
+ // IsResourceWebAccessible already does the manifest version check, so no |
+ // need to explicitly do it. |
+ if (!extensions::WebAccessibleResourcesInfo::IsResourceWebAccessible( |
+ extension, request->url().path()) && |
+ extensions::ManifestURL::GetDevToolsPage(extension).is_empty()) { |
+ return false; |
+ } |
+ |
+ if (!content::PageTransitionIsWebTriggerable(info->GetPageTransition())) |
+ return false; |
+ |
return true; |
} |
@@ -322,17 +373,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(); |
@@ -357,8 +409,9 @@ ExtensionProtocolHandler::MaybeCreateJob( |
extensions::WebAccessibleResourcesInfo::HasWebAccessibleResources( |
extension)) && |
extensions::WebAccessibleResourcesInfo::IsResourceWebAccessible( |
- extension, resource_path)) |
+ extension, resource_path)) { |
Charlie Reis
2013/02/22 02:45:17
nit: Technically this is legal by the style guide,
nasko
2013/02/22 17:14:35
Done.
|
send_cors_header = true; |
+ } |
} |
std::string path = request->url().path(); |