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

Unified Diff: extensions/browser/extension_navigation_throttle.cc

Issue 2411693003: Move blocking of top-level navigations to nested URLs with extension origins from non-extension pro… (Closed)
Patch Set: review comments Created 4 years, 2 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: extensions/browser/extension_navigation_throttle.cc
diff --git a/extensions/browser/extension_navigation_throttle.cc b/extensions/browser/extension_navigation_throttle.cc
index bdf1a27af56d003b7374132a946fb4c715992407..7223c47d008e02ac58d1ac2f8e9d922426b02d79 100644
--- a/extensions/browser/extension_navigation_throttle.cc
+++ b/extensions/browser/extension_navigation_throttle.cc
@@ -7,13 +7,17 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/url_constants.h"
#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/process_map.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_set.h"
#include "extensions/common/manifest_handlers/web_accessible_resources_info.h"
+#include "extensions/common/permissions/api_permission.h"
+#include "extensions/common/permissions/permissions_data.h"
namespace extensions {
@@ -26,14 +30,52 @@ ExtensionNavigationThrottle::~ExtensionNavigationThrottle() {}
content::NavigationThrottle::ThrottleCheckResult
ExtensionNavigationThrottle::WillStartRequest() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ GURL url(navigation_handle()->GetURL());
+ content::BrowserContext* browser_context =
+ navigation_handle()->GetWebContents()->GetBrowserContext();
+ ExtensionRegistry* registry = ExtensionRegistry::Get(browser_context);
+
+ if (navigation_handle()->IsInMainFrame()) {
+ // Block top-level navigations to blob: or filesystem: URLs with extension
+ // origin from non-extension processes. See https://crbug.com/645028.
+ bool is_nested_url = url.SchemeIsFileSystem() || url.SchemeIsBlob();
+ bool is_extension = false;
+ if (registry) {
+ is_extension = !!registry->enabled_extensions().GetExtensionOrAppByURL(
+ navigation_handle()->GetCreatorSiteInstance()->GetSiteURL());
+ if (is_extension) {
+ int child_id = navigation_handle()
+ ->GetCreatorSiteInstance()
+ ->GetProcess()
+ ->GetID();
+ DCHECK(ProcessMap::Get(browser_context)->Contains(child_id));
+ }
+ }
+
+ url::Origin origin(url);
+ if (is_nested_url && origin.scheme() == extensions::kExtensionScheme &&
+ !is_extension) {
+ // Relax this restriction for apps that use <webview>. See
+ // https://crbug.com/652077.
+ const extensions::Extension* extension =
+ registry->enabled_extensions().GetByID(origin.host());
+ bool has_webview_permission =
+ extension &&
+ extension->permissions_data()->HasAPIPermission(
+ extensions::APIPermission::kWebView);
+ if (!has_webview_permission)
+ return content::NavigationThrottle::CANCEL;
+ }
+
+ return content::NavigationThrottle::PROCEED;
+ }
- // This method for now enforces only web_accessible_resources for navigations.
- // Top-level navigations should always be allowed.
- DCHECK(!navigation_handle()->IsInMainFrame());
+ // Now enforce web_accessible_resources for navigations. Top-level navigations
+ // should always be allowed.
// If the navigation is not to a chrome-extension:// URL, no need to perform
// any more checks.
- if (!navigation_handle()->GetURL().SchemeIs(extensions::kExtensionScheme))
+ if (!url.SchemeIs(extensions::kExtensionScheme))
return content::NavigationThrottle::PROCEED;
// The subframe which is navigated needs to have all of its ancestors be
@@ -58,8 +100,7 @@ ExtensionNavigationThrottle::WillStartRequest() {
content::RenderFrameHost* ancestor = navigating_frame->GetParent();
bool external_ancestor = false;
while (ancestor) {
- if (ancestor->GetLastCommittedURL().GetOrigin() !=
- navigation_handle()->GetURL().GetOrigin()) {
+ if (ancestor->GetLastCommittedURL().GetOrigin() != url.GetOrigin()) {
// Ignore DevTools, as it is allowed to embed extension pages.
if (!ancestor->GetLastCommittedURL().SchemeIs(
content::kChromeDevToolsScheme)) {
@@ -75,15 +116,12 @@ ExtensionNavigationThrottle::WillStartRequest() {
// Since there was at least one origin different than the navigation URL,
// explicitly check for the resource in web_accessible_resources.
- std::string resource_path = navigation_handle()->GetURL().path();
- ExtensionRegistry* registry = ExtensionRegistry::Get(
- navigation_handle()->GetWebContents()->GetBrowserContext());
+ std::string resource_path = url.path();
if (!registry)
return content::NavigationThrottle::BLOCK_REQUEST;
const extensions::Extension* extension =
- registry->enabled_extensions().GetByID(
- navigation_handle()->GetURL().host());
+ registry->enabled_extensions().GetByID(url.host());
if (!extension)
return content::NavigationThrottle::BLOCK_REQUEST;
« content/public/browser/navigation_handle.h ('K') | « content/public/browser/navigation_handle.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698