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

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

Issue 2222723002: Avoid calling into the ContentBrowserClient interface from ResourceDispatcherHostImpl to determine … (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Attempt to fix 64 bit windows redness Created 4 years, 4 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/browser/extensions/chrome_content_browser_client_extensions_part.cc
diff --git a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
index 8adeb96f87829a4b0395e697e12325fb4b7ac245..f8f27ec010cfef75f667e6e1104651697b4a0815 100644
--- a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
+++ b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
@@ -27,12 +27,14 @@
#include "content/public/browser/browser_url_handler.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/vpn_service_proxy.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
#include "extensions/browser/api/web_request/web_request_api.h"
#include "extensions/browser/api/web_request/web_request_api_helpers.h"
+#include "extensions/browser/bad_message.h"
#include "extensions/browser/extension_host.h"
#include "extensions/browser/extension_message_filter.h"
#include "extensions/browser/extension_registry.h"
@@ -125,6 +127,78 @@ RenderProcessHostPrivilege GetProcessPrivilege(
return PRIV_EXTENSION;
}
+// Determines whether the extension |origin| passed in can be committed by
+// the process identified by |child_id| and returns true or false
+// accordingly. Please refer to the implementation for more information.
+bool IsIllegalOrigin(content::ResourceContext* resource_context,
+ int child_id,
+ const GURL& origin) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ // Consider non-extension URLs safe; they will be checked elsewhere.
+ if (!origin.SchemeIs(kExtensionScheme))
+ return false;
+
+ // If there is no extension installed for the URL, it couldn't have
+ // committed.
Charlie Reis 2016/08/10 20:44:01 nit: Does "committed" fit on the previous line? I
+ // (If the extension was recently uninstalled, the tab would have closed.)
+ ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
+ InfoMap* extension_info_map = io_data->GetExtensionInfoMap();
+ const Extension* extension =
+ extension_info_map->extensions().GetExtensionOrAppByURL(origin);
+ if (!extension)
+ return true;
+
+ // Check for platform app origins. These can only be committed by the app
+ // itself, or by one if its guests if there are accessible_resources.
+ const ProcessMap& process_map = extension_info_map->process_map();
+ if (extension->is_platform_app() &&
+ !process_map.Contains(extension->id(), child_id)) {
+ // This is a platform app origin not in the app's own process. If there
+ // are
Charlie Reis 2016/08/10 20:44:02 nit: Fix line wrap.
+ // no accessible resources, this is illegal.
+ if (!extension->GetManifestData(manifest_keys::kWebviewAccessibleResources))
+ return true;
+
+ // If there are accessible resources, the origin is only legal if the
+ // given
Charlie Reis 2016/08/10 20:44:01 Same.
+ // process is a guest of the app.
+ std::string owner_extension_id;
+ int owner_process_id;
+ WebViewRendererState::GetInstance()->GetOwnerInfo(
+ child_id, &owner_process_id, &owner_extension_id);
+ const Extension* owner_extension =
+ extension_info_map->extensions().GetByID(owner_extension_id);
+ return !owner_extension || owner_extension != extension;
+ }
+
+ // With only the origin and not the full URL, we don't have enough
+ // information
Charlie Reis 2016/08/10 20:44:02 Same.
+ // to validate hosted apps or web_accessible_resources in normal extensions.
+ // Assume they're legal.
+ return false;
+}
+
+// This callback is registered on the ResourceDispatcherHost for the chrome
+// extension Origin scheme. We determine whether the extension origin is
+// valid. Please see the IsIllegalOrigin() function.
+void OnHttpHeaderReceived(const std::string& header,
+ const std::string& value,
+ int child_id,
+ content::ResourceContext* resource_context,
+ content::OnHeaderProcessedCallback callback) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ GURL origin(value);
+ DCHECK(origin.SchemeIs(extensions::kExtensionScheme));
+
+ if (IsIllegalOrigin(resource_context, child_id, origin)) {
+ callback.Run(false, bad_message::INVALID_ORIGIN);
+ } else {
+ callback.Run(true, 0);
+ }
+}
+
} // namespace
ChromeContentBrowserClientExtensionsPart::
@@ -272,52 +346,6 @@ bool ChromeContentBrowserClientExtensionsPart::CanCommitURL(
return true;
}
-bool ChromeContentBrowserClientExtensionsPart::IsIllegalOrigin(
- content::ResourceContext* resource_context,
- int child_process_id,
- const GURL& origin) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
- // Consider non-extension URLs safe; they will be checked elsewhere.
- if (!origin.SchemeIs(kExtensionScheme))
- return false;
-
- // If there is no extension installed for the URL, it couldn't have committed.
- // (If the extension was recently uninstalled, the tab would have closed.)
- ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
- InfoMap* extension_info_map = io_data->GetExtensionInfoMap();
- const Extension* extension =
- extension_info_map->extensions().GetExtensionOrAppByURL(origin);
- if (!extension)
- return true;
-
- // Check for platform app origins. These can only be committed by the app
- // itself, or by one if its guests if there are accessible_resources.
- const ProcessMap& process_map = extension_info_map->process_map();
- if (extension->is_platform_app() &&
- !process_map.Contains(extension->id(), child_process_id)) {
- // This is a platform app origin not in the app's own process. If there are
- // no accessible resources, this is illegal.
- if (!extension->GetManifestData(manifest_keys::kWebviewAccessibleResources))
- return true;
-
- // If there are accessible resources, the origin is only legal if the given
- // process is a guest of the app.
- std::string owner_extension_id;
- int owner_process_id;
- WebViewRendererState::GetInstance()->GetOwnerInfo(
- child_process_id, &owner_process_id, &owner_extension_id);
- const Extension* owner_extension =
- extension_info_map->extensions().GetByID(owner_extension_id);
- return !owner_extension || owner_extension != extension;
- }
-
- // With only the origin and not the full URL, we don't have enough information
- // to validate hosted apps or web_accessible_resources in normal extensions.
- // Assume they're legal.
- return false;
-}
-
// static
bool ChromeContentBrowserClientExtensionsPart::IsSuitableHost(
Profile* profile,
@@ -662,4 +690,9 @@ void ChromeContentBrowserClientExtensionsPart::
}
}
+void ChromeContentBrowserClientExtensionsPart::ResourceDispatcherHostCreated() {
+ content::ResourceDispatcherHost::Get()->RegisterInterceptor(
+ "Origin", kExtensionScheme, base::Bind(&OnHttpHeaderReceived));
+}
+
} // namespace extensions

Powered by Google App Engine
This is Rietveld 408576698