Commit 77d532c2 authored by Kevin McNee's avatar Kevin McNee Committed by Commit Bot

Limit the use of error-providing GuestView custom elements to relevant contexts

Evidenced by issue 1014385, extensions may be using the names of
GuestViews for other purposes when a GuestView element is not available.
In order to avoid name collisions with user code, we no longer define
the error-providing elements in contexts where it is not possible to
declare the necessary permission.

Bug: 1014385
Change-Id: Icdf764c2bb19165cc173226e74e6f71eb7c99e1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869399Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarJames MacLean <wjmaclean@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712586}
parent 741b300e
...@@ -693,6 +693,7 @@ std::vector<Dispatcher::JsResourceInfo> Dispatcher::GetJsResources() { ...@@ -693,6 +693,7 @@ std::vector<Dispatcher::JsResourceInfo> Dispatcher::GetJsResources() {
std::vector<JsResourceInfo> resources = { std::vector<JsResourceInfo> resources = {
{"appView", IDR_APP_VIEW_JS}, {"appView", IDR_APP_VIEW_JS},
{"appViewElement", IDR_APP_VIEW_ELEMENT_JS}, {"appViewElement", IDR_APP_VIEW_ELEMENT_JS},
{"appViewDeny", IDR_APP_VIEW_DENY_JS},
{"entryIdManager", IDR_ENTRY_ID_MANAGER}, {"entryIdManager", IDR_ENTRY_ID_MANAGER},
{"extensionOptions", IDR_EXTENSION_OPTIONS_JS}, {"extensionOptions", IDR_EXTENSION_OPTIONS_JS},
{"extensionOptionsElement", IDR_EXTENSION_OPTIONS_ELEMENT_JS}, {"extensionOptionsElement", IDR_EXTENSION_OPTIONS_ELEMENT_JS},
...@@ -723,6 +724,7 @@ std::vector<Dispatcher::JsResourceInfo> Dispatcher::GetJsResources() { ...@@ -723,6 +724,7 @@ std::vector<Dispatcher::JsResourceInfo> Dispatcher::GetJsResources() {
{"webView", IDR_WEB_VIEW_JS}, {"webView", IDR_WEB_VIEW_JS},
{"webViewElement", IDR_WEB_VIEW_ELEMENT_JS}, {"webViewElement", IDR_WEB_VIEW_ELEMENT_JS},
{"extensionsWebViewElement", IDR_EXTENSIONS_WEB_VIEW_ELEMENT_JS}, {"extensionsWebViewElement", IDR_EXTENSIONS_WEB_VIEW_ELEMENT_JS},
{"webViewDeny", IDR_WEB_VIEW_DENY_JS},
{"webViewActionRequests", IDR_WEB_VIEW_ACTION_REQUESTS_JS}, {"webViewActionRequests", IDR_WEB_VIEW_ACTION_REQUESTS_JS},
{"webViewApiMethods", IDR_WEB_VIEW_API_METHODS_JS}, {"webViewApiMethods", IDR_WEB_VIEW_API_METHODS_JS},
{"webViewAttributes", IDR_WEB_VIEW_ATTRIBUTES_JS}, {"webViewAttributes", IDR_WEB_VIEW_ATTRIBUTES_JS},
...@@ -1385,6 +1387,23 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) { ...@@ -1385,6 +1387,23 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) {
ModuleSystem* module_system = context->module_system(); ModuleSystem* module_system = context->module_system();
bool requires_guest_view_module = false; bool requires_guest_view_module = false;
// This determines whether to register error-providing custom elements for the
// GuestView types that are not available. We only do this in contexts where
// it is possible to gain access to a given GuestView element by declaring the
// necessary permission in a manifest file. We don't want to define
// error-providing elements in other extension contexts as the names could
// collide with names used in the extension. Also, WebUIs may be whitelisted
// to use GuestViews, but we don't define the error-providing elements in this
// case.
const bool is_platform_app =
context->context_type() == Feature::BLESSED_EXTENSION_CONTEXT &&
!context->IsForServiceWorker() && context->extension() &&
context->extension()->is_platform_app();
const bool app_view_permission_exists = is_platform_app;
// The webview permission is also available to internal whitelisted
// extensions, but not to extensions in general.
const bool web_view_permission_exists = is_platform_app;
// TODO(fsamuel): Eagerly calling Require on context startup is expensive. // TODO(fsamuel): Eagerly calling Require on context startup is expensive.
// It would be better if there were a light way of detecting when a webview // It would be better if there were a light way of detecting when a webview
// or appview is created and only then set up the infrastructure. // or appview is created and only then set up the infrastructure.
...@@ -1393,6 +1412,8 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) { ...@@ -1393,6 +1412,8 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) {
if (context->GetAvailability("appViewEmbedderInternal").is_available()) { if (context->GetAvailability("appViewEmbedderInternal").is_available()) {
requires_guest_view_module = true; requires_guest_view_module = true;
module_system->Require("appViewElement"); module_system->Require("appViewElement");
} else if (app_view_permission_exists) {
module_system->Require("appViewDeny");
} }
// Require ExtensionOptions. // Require ExtensionOptions.
...@@ -1407,6 +1428,8 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) { ...@@ -1407,6 +1428,8 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) {
// The embedder of the extensions layer may define its own implementation // The embedder of the extensions layer may define its own implementation
// of WebView. // of WebView.
delegate_->RequireWebViewModules(context); delegate_->RequireWebViewModules(context);
} else if (web_view_permission_exists) {
module_system->Require("webViewDeny");
} }
if (requires_guest_view_module && if (requires_guest_view_module &&
...@@ -1425,15 +1448,6 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) { ...@@ -1425,15 +1448,6 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) {
->GetSettings() ->GetSettings()
->SetForceMainWorldInitialization(true); ->SetForceMainWorldInitialization(true);
} }
// The "guestViewDeny" module must always be loaded last. It registers
// error-providing custom elements for the GuestView types that are not
// available, and thus all of those types must have been checked and loaded
// (or not loaded) beforehand.
if (context->context_type() == Feature::BLESSED_EXTENSION_CONTEXT &&
!context->IsForServiceWorker()) {
module_system->Require("guestViewDeny");
}
} }
std::unique_ptr<NativeExtensionBindingsSystem> Dispatcher::CreateBindingsSystem( std::unique_ptr<NativeExtensionBindingsSystem> Dispatcher::CreateBindingsSystem(
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
<includes> <includes>
<!-- Extension libraries. --> <!-- Extension libraries. -->
<include name="IDR_APP_VIEW_JS" file="guest_view/app_view/app_view.js" type="BINDATA" /> <include name="IDR_APP_VIEW_JS" file="guest_view/app_view/app_view.js" type="BINDATA" />
<include name="IDR_APP_VIEW_DENY_JS" file="guest_view/app_view/app_view_deny.js" type="BINDATA" />
<include name="IDR_APP_VIEW_ELEMENT_JS" file="guest_view/app_view/app_view_element.js" type="BINDATA" /> <include name="IDR_APP_VIEW_ELEMENT_JS" file="guest_view/app_view/app_view_element.js" type="BINDATA" />
<include name="IDR_BROWSER_TEST_ENVIRONMENT_SPECIFIC_BINDINGS_JS" file="browser_test_environment_specific_bindings.js" type="BINDATA" /> <include name="IDR_BROWSER_TEST_ENVIRONMENT_SPECIFIC_BINDINGS_JS" file="browser_test_environment_specific_bindings.js" type="BINDATA" />
<include name="IDR_ENTRY_ID_MANAGER" file="entry_id_manager.js" type="BINDATA" /> <include name="IDR_ENTRY_ID_MANAGER" file="entry_id_manager.js" type="BINDATA" />
...@@ -45,6 +46,7 @@ ...@@ -45,6 +46,7 @@
<include name="IDR_WEB_VIEW_EVENTS_JS" file="guest_view/web_view/web_view_events.js" type="BINDATA" /> <include name="IDR_WEB_VIEW_EVENTS_JS" file="guest_view/web_view/web_view_events.js" type="BINDATA" />
<include name="IDR_WEB_VIEW_INTERNAL_CUSTOM_BINDINGS_JS" file="guest_view/web_view/web_view_internal.js" type="BINDATA" /> <include name="IDR_WEB_VIEW_INTERNAL_CUSTOM_BINDINGS_JS" file="guest_view/web_view/web_view_internal.js" type="BINDATA" />
<include name="IDR_WEB_VIEW_JS" file="guest_view/web_view/web_view.js" type="BINDATA" /> <include name="IDR_WEB_VIEW_JS" file="guest_view/web_view/web_view.js" type="BINDATA" />
<include name="IDR_WEB_VIEW_DENY_JS" file="guest_view/web_view/web_view_deny.js" type="BINDATA" />
<include name="IDR_WEB_VIEW_ELEMENT_JS" file="guest_view/web_view/web_view_element.js" type="BINDATA" /> <include name="IDR_WEB_VIEW_ELEMENT_JS" file="guest_view/web_view/web_view_element.js" type="BINDATA" />
<!-- Custom bindings for APIs. --> <!-- Custom bindings for APIs. -->
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var registerDeniedElement = require('guestViewDeny').registerDeniedElement;
registerDeniedElement('AppView', 'appview');
...@@ -11,48 +11,41 @@ var $CustomElementRegistry = ...@@ -11,48 +11,41 @@ var $CustomElementRegistry =
var $EventTarget = require('safeMethods').SafeMethods.$EventTarget; var $EventTarget = require('safeMethods').SafeMethods.$EventTarget;
var GuestViewInternalNatives = requireNative('guest_view_internal'); var GuestViewInternalNatives = requireNative('guest_view_internal');
var ERROR_MESSAGE = 'You do not have permission to use the %1 element.' + function registerDeniedElementInternal(viewType, permissionName) {
' Be sure to declare the "%1" permission in your manifest file.';
// A list of view types that will have custom elements registered if they are
// not already registered by the time this module is loaded.
var VIEW_TYPES = [
'AppView',
'ExtensionOptions',
'WebView'
];
// Registers a GuestView custom element.
function registerGuestViewElement(viewType) {
GuestViewInternalNatives.AllowGuestViewElementDefinition(() => { GuestViewInternalNatives.AllowGuestViewElementDefinition(() => {
var DeniedElement = class extends HTMLElement { var DeniedElement = class extends HTMLElement {
constructor() { constructor() {
super(); super();
window.console.error($String.replace( window.console.error(`You do not have permission to use the ${
ERROR_MESSAGE, /%1/g, $String.toLowerCase(viewType))); viewType} element. Be sure to declare the "${
permissionName}" permission in your manifest file.`);
} }
} }
$CustomElementRegistry.define( $CustomElementRegistry.define(
window.customElements, $String.toLowerCase(viewType), DeniedElement); window.customElements, $String.toLowerCase(viewType), DeniedElement);
// User code that does not have permission for this GuestView could be
// using the same name for another purpose, in which case we won't overwrite
// with the error-providing element.
if (!$Object.hasOwnProperty(window, viewType)) {
$Object.defineProperty(window, viewType, { $Object.defineProperty(window, viewType, {
value: DeniedElement, value: DeniedElement,
}); });
}
}); });
} }
var useCapture = true; // Registers an error-providing GuestView custom element.
window.addEventListener('readystatechange', function listener(event) { function registerDeniedElement(viewType, permissionName) {
let useCapture = true;
window.addEventListener('readystatechange', function listener(event) {
if (document.readyState == 'loading') if (document.readyState == 'loading')
return; return;
for (var viewType of VIEW_TYPES) { registerDeniedElementInternal(viewType, permissionName);
// Register the error-providing custom element only for those view types
// that have not already been registered. Since this module is always loaded
// last, all the view types that are available (i.e. have the proper
// permissions) will have already been registered on |window|.
if (!$Object.hasOwnProperty(window, viewType))
registerGuestViewElement(viewType);
}
$EventTarget.removeEventListener(window, event.type, listener, useCapture); $EventTarget.removeEventListener(window, event.type, listener, useCapture);
}, useCapture); }, useCapture);
}
// Exports.
exports.$set('registerDeniedElement', registerDeniedElement);
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
var registerDeniedElement = require('guestViewDeny').registerDeniedElement;
registerDeniedElement('WebView', 'webview');
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