Further hardening of GuestView against tainted methods and properties

In the GuestView implementation, we inadvertently call methods and
invoke property getters and setters that user code can control.

To avoid method calls to user code, we introduce safe_methods.js
which provides references to the untainted methods.

To avoid invoking user controlled getters and setters, we prevent our
internal objects from inheriting from Object.prototype or we explicitly
access own properties.

Bug: 701034, 803668, 892886
Change-Id: Ie615d6f796793313ed2db6089e259573ad25a90d
Reviewed-on: https://chromium-review.googlesource.com/c/1302698
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611832}

extensions/browser/guest_view/web_view/web_view_apitest.cc

@@ -42,6 +42,7 @@
 #include "extensions/shell/browser/shell_extension_system.h"
 #include "extensions/shell/test/shell_test.h"
 #include "extensions/test/extension_test_message_listener.h"
+#include "extensions/test/result_catcher.h"
 #include "net/base/filename_util.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
@@ -174,16 +175,26 @@ content::WebContents* WebViewAPITest::GetFirstAppWindowWebContents() {
 }
 
 void WebViewAPITest::RunTest(const std::string& test_name,
-                             const std::string& app_location) {
+                             const std::string& app_location,
+                             bool ad_hoc_framework) {
   LaunchApp(app_location);
 
-  ExtensionTestMessageListener done_listener("TEST_PASSED", false);
-  done_listener.set_failure_message("TEST_FAILED");
-  ASSERT_TRUE(content::ExecuteScript(
-      embedder_web_contents_,
-      base::StringPrintf("runTest('%s')", test_name.c_str())))
-      << "Unable to start test.";
-  ASSERT_TRUE(done_listener.WaitUntilSatisfied());
+  if (ad_hoc_framework) {
+    ExtensionTestMessageListener done_listener("TEST_PASSED", false);
+    done_listener.set_failure_message("TEST_FAILED");
+    ASSERT_TRUE(content::ExecuteScript(
+        embedder_web_contents_,
+        base::StringPrintf("runTest('%s')", test_name.c_str())))
+        << "Unable to start test.";
+    ASSERT_TRUE(done_listener.WaitUntilSatisfied());
+  } else {
+    ResultCatcher catcher;
+    ASSERT_TRUE(content::ExecuteScript(
+        embedder_web_contents_,
+        base::StringPrintf("runTest('%s')", test_name.c_str())))
+        << "Unable to start test.";
+    ASSERT_TRUE(catcher.GetNextResult()) << catcher.message();
+  }
 }
 
 void WebViewAPITest::SetUpCommandLine(base::CommandLine* command_line) {
@@ -799,4 +810,26 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestCaptureVisibleRegion) {
   RunTest("testCaptureVisibleRegion", "web_view/apitest");
 }
 
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNoUserCodeCreate) {
+  RunTest("testCreate", "web_view/no_internal_calls_to_user_code", false);
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNoUserCodeSetOnEventProperty) {
+  RunTest("testSetOnEventProperty", "web_view/no_internal_calls_to_user_code",
+          false);
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNoUserCodeGetSetAttributes) {
+  RunTest("testGetSetAttributes", "web_view/no_internal_calls_to_user_code",
+          false);
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNoUserCodeBackForward) {
+  RunTest("testBackForward", "web_view/no_internal_calls_to_user_code", false);
+}
+
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestNoUserCodeFocus) {
+  RunTest("testFocus", "web_view/no_internal_calls_to_user_code", false);
+}
+
 }  // namespace extensions

extensions/browser/guest_view/web_view/web_view_apitest.h

@@ -30,7 +30,12 @@ class WebViewAPITest : public AppShellTest {
 
   // Runs the test |test_name| in |app_location|. RunTest will launch the app
   // and execute the javascript function runTest(test_name) inside the app.
-  void RunTest(const std::string& test_name, const std::string& app_location);
+  // If |ad_hoc_framework| is true, the test app defines its own testing
+  // framework, otherwise the test app uses the chrome.test framework.
+  // See https://crbug.com/876330
+  void RunTest(const std::string& test_name,
+               const std::string& app_location,
+               bool ad_hoc_framework = true);
 
   // Starts/Stops the embedded test server.
   void StartTestServer(const std::string& app_location);

extensions/renderer/BUILD.gn

@@ -232,6 +232,7 @@ jumbo_source_set("renderer") {
     "resources/guest_view/guest_view_events.js",
     "resources/guest_view/guest_view_iframe.js",
     "resources/guest_view/guest_view_iframe_container.js",
+    "resources/guest_view/safe_methods.js",
     "resources/guest_view/web_view/extensions_web_view_element.js",
     "resources/guest_view/web_view/web_view.js",
     "resources/guest_view/web_view/web_view_action_requests.js",

extensions/renderer/dispatcher.cc

@@ -652,6 +652,7 @@ std::vector<Dispatcher::JsResourceInfo> Dispatcher::GetJsResources() {
       {"guestViewContainerElement", IDR_GUEST_VIEW_CONTAINER_ELEMENT_JS},
       {"guestViewDeny", IDR_GUEST_VIEW_DENY_JS},
       {"guestViewEvents", IDR_GUEST_VIEW_EVENTS_JS},
+      {"safeMethods", IDR_SAFE_METHODS_JS},
       {"imageUtil", IDR_IMAGE_UTIL_JS},
       {"setIcon", IDR_SET_ICON_JS},
       {"test", IDR_TEST_CUSTOM_BINDINGS_JS},

extensions/renderer/resources/extensions_renderer_resources.grd

@@ -45,6 +45,7 @@
       <include name="IDR_MESSAGING_UTILS_JS" file="messaging_utils.js" type="BINDATA" />
       <include name="IDR_MIME_HANDLER_PRIVATE_CUSTOM_BINDINGS_JS" file="mime_handler_private_custom_bindings.js" type="BINDATA" />
       <include name="IDR_MIME_HANDLER_MOJOM_JS" file="${mojom_root}\extensions\common\api\mime_handler.mojom.js" use_base_dir="false" type="BINDATA" />
+      <include name="IDR_SAFE_METHODS_JS" file="guest_view/safe_methods.js" type="BINDATA" />
       <include name="IDR_SCHEMA_UTILS_JS" file="schema_utils.js" type="BINDATA" />
       <include name="IDR_SEND_REQUEST_JS" file="send_request.js" type="BINDATA" />
       <include name="IDR_SET_ICON_JS" file="set_icon.js" type="BINDATA" />

extensions/renderer/resources/guest_view/app_view/app_view.js

@@ -2,6 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+var $Document = require('safeMethods').SafeMethods.$Document;
+var $HTMLElement = require('safeMethods').SafeMethods.$HTMLElement;
+var $Node = require('safeMethods').SafeMethods.$Node;
 var GuestViewContainer = require('guestViewContainer').GuestViewContainer;
 
 function AppViewImpl(appviewElement) {
@@ -15,23 +18,24 @@ AppViewImpl.prototype.__proto__ = GuestViewContainer.prototype;
 
 AppViewImpl.prototype.getErrorNode = function() {
   if (!this.errorNode) {
-    this.errorNode = document.createElement('div');
-    this.errorNode.innerText = 'Unable to connect to app.';
-    this.errorNode.style.position = 'absolute';
-    this.errorNode.style.left = '0px';
-    this.errorNode.style.top = '0px';
-    this.errorNode.style.width = '100%';
-    this.errorNode.style.height = '100%';
-    this.element.shadowRoot.appendChild(this.errorNode);
+    this.errorNode = $Document.createElement(document, 'div');
+    $HTMLElement.innerText.set(this.errorNode, 'Unable to connect to app.');
+    var style = $HTMLElement.style.get(this.errorNode);
+    $Object.defineProperty(style, 'position', {value: 'absolute'});
+    $Object.defineProperty(style, 'left', {value: '0px'});
+    $Object.defineProperty(style, 'top', {value: '0px'});
+    $Object.defineProperty(style, 'width', {value: '100%'});
+    $Object.defineProperty(style, 'height', {value: '100%'});
+    $Node.appendChild(this.shadowRoot, this.errorNode);
   }
   return this.errorNode;
 };
 
 AppViewImpl.prototype.buildContainerParams = function() {
-  return {
-    'appId': this.app,
-    'data': this.data || {}
-  };
+  var params = $Object.create(null);
+  params.appId = this.app;
+  params.data = this.data || {};
+  return params;
 };
 
 AppViewImpl.prototype.connect = function(app, data, callback) {
@@ -50,7 +54,7 @@ AppViewImpl.prototype.connect = function(app, data, callback) {
     if (!this.guest.getId()) {
       var errorMsg = 'Unable to connect to app "' + app + '".';
       window.console.warn(errorMsg);
-      this.getErrorNode().innerText = errorMsg;
+      $HTMLElement.innerText.set(this.getErrorNode(), errorMsg);
       if (callback) {
         callback(false);
       }

extensions/renderer/resources/guest_view/extension_options/extension_options.js

@@ -30,7 +30,7 @@ ExtensionOptionsImpl.prototype.setupAttributes = function() {
 };
 
 ExtensionOptionsImpl.prototype.buildContainerParams = function() {
-  var params = {};
+  var params = $Object.create(null);
   for (var i in this.attributes) {
     params[i] = this.attributes[i].getValue();
   }

extensions/renderer/resources/guest_view/extension_view/extension_view.js

@@ -40,7 +40,7 @@ ExtensionViewImpl.prototype.createGuest = function(callback) {
 };
 
 ExtensionViewImpl.prototype.buildContainerParams = function() {
-  var params = {};
+  var params = $Object.create(null);
   for (var i in this.attributes) {
     params[i] = this.attributes[i].getValue();
   }

extensions/renderer/resources/guest_view/guest_view.js

@@ -51,8 +51,8 @@ function GuestViewImpl(guestView, viewType, guestInstanceId) {
 
 // Prevent GuestViewImpl inadvertently inheriting code from the global Object,
 // allowing a pathway for executing unintended user code execution.
-// TODO(wjmaclean): Use utils.expose() here instead? Track down other issues
-// of Object inheritance. https://crbug.com/701034
+// TODO(wjmaclean): Track down other issues of Object inheritance.
+// https://crbug.com/701034
 GuestViewImpl.prototype.__proto__ = null;
 
 // Possible states.

extensions/renderer/resources/guest_view/guest_view_attributes.js

@@ -4,6 +4,9 @@
 
 // This module implements the base attributes of the GuestView tags.
 
+var $parseInt = require('safeMethods').SafeMethods.$parseInt;
+var $Element = require('safeMethods').SafeMethods.$Element;
+
 // -----------------------------------------------------------------------------
 // Attribute object.
 
@@ -19,13 +22,13 @@ function Attribute(name, view) {
 
 // Prevent GuestViewEvents inadvertently inheritng code from the global Object,
 // allowing a pathway for unintended execution of user code.
-// TODO(wjmaclean): Use utils.expose() here instead, track down other issues
-// of Object inheritance. https://crbug.com/701034
+// TODO(wjmaclean): Track down other issues of Object inheritance.
+// https://crbug.com/701034
 Attribute.prototype.__proto__ = null;
 
 // Retrieves and returns the attribute's value.
 Attribute.prototype.getValue = function() {
-  return this.view.element.getAttribute(this.name) || '';
+  return $Element.getAttribute(this.view.element, this.name) || '';
 };
 
 // Retrieves and returns the attribute's value if it has been dirtied since
@@ -39,7 +42,7 @@ Attribute.prototype.getValueIfDirty = function() {
 
 // Sets the attribute's value.
 Attribute.prototype.setValue = function(value) {
-  this.view.element.setAttribute(this.name, value || '');
+  $Element.setAttribute(this.view.element, this.name, value || '');
 };
 
 // Changes the attribute's value without triggering its mutation handler.
@@ -91,14 +94,14 @@ function BooleanAttribute(name, view) {
 BooleanAttribute.prototype.__proto__ = Attribute.prototype;
 
 BooleanAttribute.prototype.getValue = function() {
-  return this.view.element.hasAttribute(this.name);
+  return $Element.hasAttribute(this.view.element, this.name);
 };
 
 BooleanAttribute.prototype.setValue = function(value) {
   if (!value) {
-    this.view.element.removeAttribute(this.name);
+    $Element.removeAttribute(this.view.element, this.name);
   } else {
-    this.view.element.setAttribute(this.name, '');
+    $Element.setAttribute(this.view.element, this.name, '');
   }
 };
 
@@ -113,11 +116,11 @@ function IntegerAttribute(name, view) {
 IntegerAttribute.prototype.__proto__ = Attribute.prototype;
 
 IntegerAttribute.prototype.getValue = function() {
-  return parseInt(this.view.element.getAttribute(this.name)) || 0;
+  return $parseInt($Element.getAttribute(this.view.element, this.name)) || 0;
 };
 
 IntegerAttribute.prototype.setValue = function(value) {
-  this.view.element.setAttribute(this.name, parseInt(value) || 0);
+  $Element.setAttribute(this.view.element, this.name, $parseInt(value) || 0);
 };
 
 // -----------------------------------------------------------------------------

extensions/renderer/resources/guest_view/guest_view_container.js

@@ -9,13 +9,19 @@
 // TODO(mcnee): When BrowserPlugin is removed, merge
 // guest_view_iframe_container.js into this file.
 
+var $parseInt = require('safeMethods').SafeMethods.$parseInt;
+var $getComputedStyle = require('safeMethods').SafeMethods.$getComputedStyle;
+var $Element = require('safeMethods').SafeMethods.$Element;
+var $EventTarget = require('safeMethods').SafeMethods.$EventTarget;
+var $HTMLElement = require('safeMethods').SafeMethods.$HTMLElement;
+var $Node = require('safeMethods').SafeMethods.$Node;
 var GuestView = require('guestView').GuestView;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
 var IdGenerator = requireNative('id_generator');
 var MessagingNatives = requireNative('messaging_natives');
 
 function GuestViewContainer(element, viewType) {
-  this.attributes = {};
+  this.attributes = $Object.create(null);
   this.element = element;
   this.elementAttached = false;
   this.viewInstanceId = IdGenerator.GetNextId();
@@ -26,16 +32,16 @@ function GuestViewContainer(element, viewType) {
   this.setupAttributes();
 
   this.internalElement = this.createInternalElement$();
-  var shadowRoot = this.element.attachShadow({mode: 'open'});
-  shadowRoot.appendChild(this.internalElement);
+  this.shadowRoot = $Element.attachShadow(this.element, {mode: 'open'});
+  $Node.appendChild(this.shadowRoot, this.internalElement);
 
   GuestViewInternalNatives.RegisterView(this.viewInstanceId, this, viewType);
 }
 
 // Prevent GuestViewContainer inadvertently inheriting code from the global
 // Object, allowing a pathway for executing unintended user code execution.
-// TODO(wjmaclean): Use utils.expose() here instead? Track down other issues
-// of Object inheritance. https://crbug.com/701034
+// TODO(wjmaclean): Track down other issues of Object inheritance.
+// https://crbug.com/701034
 GuestViewContainer.prototype.__proto__ = null;
 
 // Create the 'guest' property to track new GuestViews and always listen for
@@ -77,7 +83,7 @@ GuestViewContainer.prototype.prepareForReattach$ = function() {};
 
 GuestViewContainer.prototype.focus = function() {
   // Focus the internal element when focus() is called on the GuestView element.
-  this.internalElement.focus();
+  $HTMLElement.focus(this.internalElement);
 }
 
 GuestViewContainer.prototype.attachWindow$ = function() {
@@ -117,8 +123,9 @@ GuestViewContainer.prototype.onInternalInstanceId = function(
 GuestViewContainer.prototype.handleInternalElementAttributeMutation =
     function(name, oldValue, newValue) {
   if (name == 'internalinstanceid' && !oldValue && !!newValue) {
-    this.internalElement.removeAttribute('internalinstanceid');
-    this.onInternalInstanceId(parseInt(newValue));
+    $Element.removeAttribute(
+        this.internalElement, 'internalinstanceid');
+    this.onInternalInstanceId($parseInt(newValue));
   }
 };
 
@@ -136,18 +143,18 @@ GuestViewContainer.prototype.buildParams = function() {
   // However, in the case where the GuestViewContainer has a fixed size we can
   // use that value to initially size the guest so as to avoid a relayout of the
   // on display:block.
-  var css = window.getComputedStyle(this.element, null);
-  var elementRect = this.element.getBoundingClientRect();
-  params['elementWidth'] = parseInt(elementRect.width) ||
-      parseInt(css.getPropertyValue('width'));
-  params['elementHeight'] = parseInt(elementRect.height) ||
-      parseInt(css.getPropertyValue('height'));
+  var css = $getComputedStyle(this.element, null);
+  var elementRect = $Element.getBoundingClientRect(this.element);
+  params['elementWidth'] =
+      $parseInt(elementRect.width) || $parseInt(css.getPropertyValue('width'));
+  params['elementHeight'] = $parseInt(elementRect.height) ||
+      $parseInt(css.getPropertyValue('height'));
   return params;
 };
 
 GuestViewContainer.prototype.dispatchEvent = function(event) {
-  return this.element.dispatchEvent(event);
-}
+  return $EventTarget.dispatchEvent(this.element, event);
+};
 
 // Returns a wrapper function for |func| with a weak reference to |this|.
 GuestViewContainer.prototype.weakWrapper = function(func) {
@@ -163,7 +170,9 @@ GuestViewContainer.prototype.weakWrapper = function(func) {
 GuestViewContainer.prototype.willAttachElement$ = function() {};
 
 // Implemented by the specific view type, if needed.
-GuestViewContainer.prototype.buildContainerParams = function() { return {}; };
+GuestViewContainer.prototype.buildContainerParams = function() {
+  return $Object.create(null);
+};
 GuestViewContainer.prototype.onElementAttached = function() {};
 GuestViewContainer.prototype.onElementDetached = function() {};
 GuestViewContainer.prototype.setupAttributes = function() {};

extensions/renderer/resources/guest_view/guest_view_container_element.js

@@ -5,6 +5,11 @@
 // Common custom element registration code for the various guest view
 // containers.
 
+var $CustomElementRegistry =
+    require('safeMethods').SafeMethods.$CustomElementRegistry;
+var $Element = require('safeMethods').SafeMethods.$Element;
+var $EventTarget = require('safeMethods').SafeMethods.$EventTarget;
+var $HTMLElement = require('safeMethods').SafeMethods.$HTMLElement;
 var GuestViewContainer = require('guestViewContainer').GuestViewContainer;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
 var IdGenerator = requireNative('id_generator');
@@ -21,7 +26,8 @@ function registerElement(elementName, containerElementType) {
 
     registerInternalElement($String.toLowerCase(elementName));
     registerGuestViewElement(elementName, containerElementType);
-    window.removeEventListener(event.type, listener, useCapture);
+
+    $EventTarget.removeEventListener(window, event.type, listener, useCapture);
   }, useCapture);
 }
 
@@ -36,10 +42,12 @@ function registerInternalElement(viewType) {
 
       constructor() {
         super();
-        this.setAttribute('type', 'application/browser-plugin');
-        this.setAttribute('id', 'browser-plugin-' + IdGenerator.GetNextId());
-        this.style.width = '100%';
-        this.style.height = '100%';
+        $Element.setAttribute(this, 'type', 'application/browser-plugin');
+        $Element.setAttribute(
+            this, 'id', 'browser-plugin-' + IdGenerator.GetNextId());
+        var style = $HTMLElement.style.get(this);
+        $Object.defineProperty(style, 'width', {value: '100%'});
+        $Object.defineProperty(style, 'height', {value: '100%'});
       }
     }
 
@@ -57,9 +65,12 @@ function registerInternalElement(viewType) {
       internal.handleInternalElementAttributeMutation(name, oldValue, newValue);
     };
 
-    window.customElements.define(
-        viewType + 'browserplugin', InternalElement, {extends: 'object'});
-    GuestViewContainer[viewType + 'BrowserPlugin'] = InternalElement;
+    $CustomElementRegistry.define(
+        window.customElements, viewType + 'browserplugin', InternalElement,
+        {extends: 'object'});
+    $Object.defineProperty(GuestViewContainer, viewType + 'BrowserPlugin', {
+      value: InternalElement,
+    });
 
     delete InternalElement.prototype.connectedCallback;
     delete InternalElement.prototype.attributeChangedCallback;
@@ -116,9 +127,12 @@ function registerGuestViewElement(elementName, containerElementType) {
     GuestViewContainerElement.prototype.attributeChangedCallback =
         customElementCallbacks.attributeChangedCallback;
 
-    window.customElements.define(
-        $String.toLowerCase(elementName), containerElementType);
-    window[elementName] = containerElementType;
+    $CustomElementRegistry.define(
+        window.customElements, $String.toLowerCase(elementName),
+        containerElementType);
+    $Object.defineProperty(window, elementName, {
+      value: containerElementType,
+    });
 
     delete GuestViewContainerElement.prototype.connectedCallback;
     delete GuestViewContainerElement.prototype.disconnectedCallback;

extensions/renderer/resources/guest_view/guest_view_deny.js

@@ -6,6 +6,9 @@
 // permissions are not available. These elements exist only to provide a useful
 // error message when developers attempt to use them.
 
+var $CustomElementRegistry =
+    require('safeMethods').SafeMethods.$CustomElementRegistry;
+var $EventTarget = require('safeMethods').SafeMethods.$EventTarget;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
 
 var ERROR_MESSAGE = 'You do not have permission to use the %1 element.' +
@@ -30,9 +33,11 @@ function registerGuestViewElement(viewType) {
             ERROR_MESSAGE, /%1/g, $String.toLowerCase(viewType)));
       }
     }
-
-    window.customElements.define($String.toLowerCase(viewType), DeniedElement);
-    window[viewType] = DeniedElement;
+    $CustomElementRegistry.define(
+        window.customElements, $String.toLowerCase(viewType), DeniedElement);
+    $Object.defineProperty(window, viewType, {
+      value: DeniedElement,
+    });
   });
 }
 
@@ -46,9 +51,9 @@ window.addEventListener('readystatechange', function listener(event) {
     // 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 (!window[viewType])
+    if (!$Object.hasOwnProperty(window, viewType))
       registerGuestViewElement(viewType);
   }
 
-  window.removeEventListener(event.type, listener, useCapture);
+  $EventTarget.removeEventListener(window, event.type, listener, useCapture);
 }, useCapture);

extensions/renderer/resources/guest_view/guest_view_events.js

@@ -4,6 +4,7 @@
 
 // Event management for GuestViewContainers.
 
+var $EventTarget = require('safeMethods').SafeMethods.$EventTarget;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
 var MessagingNatives = requireNative('messaging_natives');
 
@@ -30,7 +31,7 @@ function GuestViewEvents(view) {
   view.events = this;
 
   this.view = view;
-  this.on = {};
+  this.on = $Object.create(null);
 
   // |setupEventProperty| is normally called automatically, but these events are
   // are registered here because they are dispatched from GuestViewContainer
@@ -42,8 +43,8 @@ function GuestViewEvents(view) {
 
 // Prevent GuestViewEvents inadvertently inheritng code from the global Object,
 // allowing a pathway for unintended execution of user code.
-// TODO(wjmaclean): Use utils.expose() here instead, track down other issues
-// of Object inheritance. https://crbug.com/701034
+// TODO(wjmaclean): Track down other issues of Object inheritance.
+// https://crbug.com/701034
 GuestViewEvents.prototype.__proto__ = null;
 
 // |GuestViewEvents.EVENTS| is a dictionary of extension events to be listened
@@ -69,7 +70,7 @@ GuestViewEvents.prototype.__proto__ = null;
 //     element. A |handler| should be specified for all internal events, and
 //     |fields| and |cancelable| should be left unspecified (as they are only
 //     meaningful for DOM events).
-GuestViewEvents.EVENTS = {};
+GuestViewEvents.EVENTS = $Object.create(null);
 
 // Attaches |listener| onto the event descriptor object |evt|, and registers it
 // to be removed once this GuestViewEvents object is garbage collected.
@@ -141,7 +142,8 @@ GuestViewEvents.prototype.makeDomEvent = function(event, eventName) {
     return null;
   }
 
-  var details = { bubbles: true };
+  var details = $Object.create(null);
+  details.bubbles = true;
   if (eventInfo.cancelable) {
     details.cancelable = true;
   }
@@ -149,7 +151,7 @@ GuestViewEvents.prototype.makeDomEvent = function(event, eventName) {
   if (eventInfo.fields) {
     $Array.forEach(eventInfo.fields, $Function.bind(function(field) {
       if (event[field] !== undefined) {
-        domEvent[field] = event[field];
+        $Object.defineProperty(domEvent, field, {value: event[field]});
       }
     }, this));
   }
@@ -167,11 +169,12 @@ GuestViewEvents.prototype.setupEventProperty = function(eventName) {
     }, this),
     set: $Function.bind(function(value) {
       if (this.on[propertyName]) {
-        this.view.element.removeEventListener(eventName, this.on[propertyName]);
+        $EventTarget.removeEventListener(
+            this.view.element, eventName, this.on[propertyName]);
       }
       this.on[propertyName] = value;
       if (value) {
-        this.view.element.addEventListener(eventName, value);
+        $EventTarget.addEventListener(this.view.element, eventName, value);
       }
     }, this),
     enumerable: true

extensions/renderer/resources/guest_view/guest_view_iframe.js

@@ -4,6 +4,7 @@
 
 // GuestViewCrossProcessFrames overrides for guest_view.js.
 
+var $HTMLIFrameElement = require('safeMethods').SafeMethods.$HTMLIFrameElement;
 var GuestViewImpl = require('guestView').GuestViewImpl;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
 var ResizeEvent = require('guestView').ResizeEvent;
@@ -15,7 +16,7 @@ var getIframeContentWindow = function(viewInstanceId) {
 
   var internalIframeElement = view.internalElement;
   if (internalIframeElement)
-    return internalIframeElement.contentWindow;
+    return $HTMLIFrameElement.contentWindow.get(internalIframeElement);
 
   return null;
 };

extensions/renderer/resources/guest_view/guest_view_iframe_container.js

@@ -4,14 +4,20 @@
 
 // GuestViewCrossProcessFrames overrides for guest_view_container.js
 
+var $Document = require('safeMethods').SafeMethods.$Document;
+var $HTMLElement = require('safeMethods').SafeMethods.$HTMLElement;
+var $Node = require('safeMethods').SafeMethods.$Node;
 var GuestViewContainer = require('guestViewContainer').GuestViewContainer;
 var IdGenerator = requireNative('id_generator');
 
 GuestViewContainer.prototype.createInternalElement$ = function() {
-  var iframeElement = document.createElement('iframe');
-  iframeElement.style.width = '100%';
-  iframeElement.style.height = '100%';
-  iframeElement.style.border = '0';
+  var iframeElement = $Document.createElement(document, 'iframe');
+
+  var style = $HTMLElement.style.get(iframeElement);
+  $Object.defineProperty(style, 'width', {value: '100%'});
+  $Object.defineProperty(style, 'height', {value: '100%'});
+  $Object.defineProperty(style, 'border', {value: '0px'});
+
   return iframeElement;
 };
 
@@ -21,7 +27,8 @@ GuestViewContainer.prototype.prepareForReattach$ = function() {
   var newFrame = this.createInternalElement$();
   var oldFrame = this.internalElement;
   this.internalElement = newFrame;
-  oldFrame.parentNode.replaceChild(newFrame, oldFrame);
+  var frameParent = $Node.parentNode.get(oldFrame);
+  $Node.replaceChild(frameParent, newFrame, oldFrame);
 };
 
 GuestViewContainer.prototype.attachWindow$ = function() {

extensions/renderer/resources/guest_view/safe_methods.js

+// Copyright (c) 2018 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.
+
+// This module keeps references to original methods before user code is able
+// to overwrite them. We assume that this module is executed before any user
+// code. The idea is similar to the extension system's SafeBuiltins, and since
+// it's similar, we also use a $ prefix as a naming convention.
+// For example,
+// myElement.setAttribute(name, value);
+// becomes
+// $Element.setAttribute(myElement, name, value);
+// We also provide access to getters and setters:
+// myNode.parentNode;
+// becomes
+// $Node.parentNode.get(myNode);
+
+function makeCallable(prototypeMethod) {
+  return (thisArg, ...args) => {
+    return $Function.apply(prototypeMethod, thisArg, args);
+  };
+}
+
+function saveMethods(original, safe, methods) {
+  for (var method of methods) {
+    safe[method] = makeCallable(original.prototype[method]);
+  }
+}
+
+function saveAccessors(original, safe, properties) {
+  for (var property of properties) {
+    var desc = $Object.getOwnPropertyDescriptor(original.prototype, property);
+
+    safe[property] = {
+      get: desc.get && makeCallable(desc.get),
+      set: desc.set && makeCallable(desc.set),
+    };
+  }
+}
+
+var SafeMethods = {
+  $CustomElementRegistry: {},
+  $Document: {},
+  $Element: {},
+  $EventTarget: {},
+  $HTMLElement: {},
+  $HTMLIFrameElement: {},
+  $MutationObserver: MutationObserver,
+  $Node: {},
+  $getComputedStyle: window.getComputedStyle,
+  $parseInt: window.parseInt,
+};
+
+saveMethods(CustomElementRegistry, SafeMethods.$CustomElementRegistry, [
+  'define',
+]);
+
+saveMethods(Document, SafeMethods.$Document, [
+  'createElement',
+  'webkitCancelFullScreen',
+]);
+
+saveMethods(Element, SafeMethods.$Element, [
+  'attachShadow',
+  'getAttribute',
+  'getBoundingClientRect',
+  'hasAttribute',
+  'removeAttribute',
+  'setAttribute',
+  'webkitRequestFullScreen',
+]);
+
+saveMethods(EventTarget, SafeMethods.$EventTarget, [
+  'addEventListener',
+  'dispatchEvent',
+  'removeEventListener',
+]);
+
+saveMethods(HTMLElement, SafeMethods.$HTMLElement, [
+  'focus',
+]);
+
+saveAccessors(HTMLElement, SafeMethods.$HTMLElement, [
+  'style',
+  'innerText',
+]);
+
+saveAccessors(HTMLIFrameElement, SafeMethods.$HTMLIFrameElement, [
+  'contentWindow',
+]);
+
+saveMethods(MutationObserver, SafeMethods.$MutationObserver, [
+  'observe',
+  'takeRecords',
+]);
+
+saveMethods(Node, SafeMethods.$Node, [
+  'appendChild',
+  'replaceChild',
+]);
+
+saveAccessors(Node, SafeMethods.$Node, [
+  'parentNode',
+]);
+
+exports.$set('SafeMethods', SafeMethods);

extensions/renderer/resources/guest_view/web_view/web_view.js

@@ -6,6 +6,7 @@
 // BrowserPlugin object element. The object element is hidden within
 // the shadow DOM of the WebView element.
 
+var $Element = require('safeMethods').SafeMethods.$Element;
 var GuestView = require('guestView').GuestView;
 var GuestViewContainer = require('guestViewContainer').GuestViewContainer;
 var GuestViewInternalNatives = requireNative('guest_view_internal');
@@ -163,10 +164,9 @@ WebViewImpl.prototype.onAttach = function(storagePartitionId) {
 };
 
 WebViewImpl.prototype.buildContainerParams = function() {
-  var params = {
-    'initialZoomFactor': this.pendingZoomFactor_,
-    'userAgentOverride': this.userAgentOverride
-  };
+  var params = $Object.create(null);
+  params.initialZoomFactor = this.pendingZoomFactor_;
+  params.userAgentOverride = this.userAgentOverride;
   for (var i in this.attributes) {
     var value = this.attributes[i].getValueIfDirty();
     if (value)
@@ -248,7 +248,7 @@ WebViewImpl.prototype.setZoom = function(zoomFactor, callback) {
 // Requests the <webview> element wihtin the embedder to enter fullscreen.
 WebViewImpl.prototype.makeElementFullscreen = function() {
   GuestViewInternalNatives.RunWithGesture($Function.bind(function() {
-    this.element.webkitRequestFullScreen();
+    $Element.webkitRequestFullScreen(this.element);
   }, this));
 };
 

extensions/renderer/resources/guest_view/web_view/web_view_action_requests.js

@@ -41,8 +41,8 @@ function WebViewActionRequest(webViewImpl, event, webViewEvent, interfaceName) {
 
 // Prevent GuestViewEvents inadvertently inheritng code from the global Object,
 // allowing a pathway for unintended execution of user code.
-// TODO(wjmaclean): Use utils.expose() here instead, track down other issues
-// of Object inheritance. https://crbug.com/701034
+// TODO(wjmaclean): Track down other issues of Object inheritance.
+// https://crbug.com/701034
 WebViewActionRequest.prototype.__proto__ = null;
 
 // Performs the default action for the request.

extensions/renderer/resources/guest_view/web_view/web_view_attributes.js

@@ -4,6 +4,8 @@
 
 // This module implements the attributes of the <webview> tag.
 
+var $Element = require('safeMethods').SafeMethods.$Element;
+var $MutationObserver = require('safeMethods').SafeMethods.$MutationObserver;
 var GuestViewAttributes = require('guestViewAttributes').GuestViewAttributes;
 var WebViewConstants = require('webViewConstants').WebViewConstants;
 var WebViewInternal = getInternalApi ?
@@ -127,9 +129,9 @@ NameAttribute.prototype.handleMutation = function(oldValue, newValue) {
 NameAttribute.prototype.setValue = function(value) {
   value = value || '';
   if (value === '')
-    this.view.element.removeAttribute(this.name);
+    $Element.removeAttribute(this.view.element, this.name);
   else
-    this.view.element.setAttribute(this.name, value);
+    $Element.setAttribute(this.view.element, this.name, value);
 };
 
 // -----------------------------------------------------------------------------
@@ -189,7 +191,7 @@ SrcAttribute.prototype.setValueIgnoreMutation = function(value) {
   // possible for this change to get picked up asyncronously by src's mutation
   // observer |observer|, and then get handled even though we do not want to
   // handle this mutation.
-  this.observer.takeRecords();
+  $MutationObserver.takeRecords(this.observer);
 };
 
 SrcAttribute.prototype.handleMutation = function(oldValue, newValue) {
@@ -218,9 +220,8 @@ SrcAttribute.prototype.detach = function() {
 // attribute without any changes to its value. This is useful in the case
 // where the webview guest has crashed and navigating to the same address
 // spawns off a new process.
-SrcAttribute.prototype.setupMutationObserver =
-    function() {
-  this.observer = new MutationObserver($Function.bind(function(mutations) {
+SrcAttribute.prototype.setupMutationObserver = function() {
+  this.observer = new $MutationObserver($Function.bind(function(mutations) {
     $Array.forEach(mutations, $Function.bind(function(mutation) {
       var oldValue = mutation.oldValue;
       var newValue = this.getValue();
@@ -235,7 +236,7 @@ SrcAttribute.prototype.setupMutationObserver =
     attributeOldValue: true,
     attributeFilter: [this.name]
   };
-  this.observer.observe(this.view.element, params);
+  $MutationObserver.observe(this.observer, this.view.element, params);
 };
 
 SrcAttribute.prototype.parse = function() {

extensions/renderer/resources/guest_view/web_view/web_view_events.js

@@ -4,6 +4,7 @@
 
 // Event management for WebView.
 
+var $Document = require('safeMethods').SafeMethods.$Document;
 var CreateEvent = require('guestViewEvents').CreateEvent;
 var DCHECK = requireNative('logging').DCHECK;
 var DeclarativeWebRequestSchema =
@@ -196,6 +197,11 @@ WebViewEvents.EVENTS = {
   }
 };
 
+WebViewEvents.EVENTS.__proto__ = null;
+for (var eventName in WebViewEvents.EVENTS) {
+  WebViewEvents.EVENTS[eventName].__proto__ = null;
+}
+
 WebViewEvents.prototype.setupWebRequestEvents = function() {
   var request = {};
   var createWebRequestEvent = $Function.bind(function(webRequestEvent) {
@@ -277,7 +283,7 @@ WebViewEvents.prototype.handleFrameNameChangedEvent = function(event) {
 };
 
 WebViewEvents.prototype.handleFullscreenExitEvent = function(event, eventName) {
-  document.webkitCancelFullScreen();
+  $Document.webkitCancelFullScreen(document);
 };
 
 WebViewEvents.prototype.handleLoadAbortEvent = function(event, eventName) {

extensions/test/data/web_view/no_internal_calls_to_user_code/main.html

+<!doctype html>
+<!--
+ * Copyright 2018 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.
+-->
+<html>
+<head>
+  <script src="main.js"></script>
+</head>
+<body>
+</body>
+</html>

extensions/test/data/web_view/no_internal_calls_to_user_code/main.js

+// Copyright 2018 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.
+
+'use strict';
+
+// Test that the webview JS implementation does not inadvertently call user
+// code.
+//
+// Our implementation should use extensions::SafeBuiltins or otherwise
+// keep references to the real methods to avoid calling overwritten methods.
+// Our internal objects can be modified to not inherit from Object in order
+// to avoid calling getters and setters if a property name is defined on
+// Object.prototype.
+//
+// Note that the properties and methods we taint are not exhaustive.
+
+window.onload = () => {
+  chrome.test.sendMessage('LAUNCHED');
+};
+
+// These are needed for the test itself, so keep a reference to the real method.
+EventTarget.prototype.savedAddEventListener =
+    EventTarget.prototype.addEventListener;
+Node.prototype.savedAppendChild = Node.prototype.appendChild;
+
+function makeUnreached() {
+  return function unreachableFunction() {
+    chrome.test.fail('Reached unreachable code');
+  };
+}
+
+(function taintProperties() {
+  var properties = [
+    'AppView',
+    'WebView',
+    '__proto__',
+    'actionQueue',
+    'allowscaling',
+    'allowtransparency',
+    'app',
+    'appview',
+    'attributes',
+    'autosize',
+    'border',
+    'cancelable',
+    'constructor',
+    'contentWindow',
+    'data',
+    'dirty',
+    'element',
+    'elementHeight',
+    'errorNode',
+    'events',
+    'guest',
+    'guestView',
+    'height',
+    'initialZoomFactor',
+    'innerText',
+    'instanceId',
+    'internal',
+    'internalInstanceId',
+    'left',
+    'listener',
+    'loadstop',
+    'maxheight',
+    'newHeight',
+    'on',
+    'onloadstop',
+    'onresize',
+    'parentNode',
+    'partition',
+    'pendingAction',
+    'position',
+    'processId',
+    'prototype',
+    'shadowRoot',
+    'src',
+    'state',
+    'style',
+    'top',
+    'userAgentOverride',
+    'validPartitionId',
+    'view',
+    'viewInstanceId',
+    'viewType',
+    'webview',
+    'webviewBrowserPlugin',
+    'webviewbrowserplugin',
+  ];
+  // For objects that don't inherit directly from Object, we'll need to taint
+  // existing properties on prototypes earlier in the prototype chain.
+  var otherConstructors = [
+    Element,
+    HTMLElement,
+    HTMLIFrameElement,
+    Node,
+  ];
+  for (var property of properties) {
+    Object.defineProperty(Object.prototype, property, {
+      get: makeUnreached(),
+      set: makeUnreached(),
+    });
+    for (var constructor of otherConstructors) {
+      if (constructor.prototype.hasOwnProperty(property)) {
+        Object.defineProperty(constructor.prototype, property, {
+          get: makeUnreached(),
+          set: makeUnreached(),
+        });
+      }
+    }
+  }
+})();
+
+// Overwrite methods.
+Object.assign = makeUnreached();
+Object.create = makeUnreached();
+Object.defineProperty = makeUnreached();
+Object.freeze = makeUnreached();
+Object.getOwnPropertyDescriptor = makeUnreached();
+Object.getPrototypeOf = makeUnreached();
+Object.keys = makeUnreached();
+Object.setPrototypeOf = makeUnreached();
+Object.prototype.hasOwnProperty = makeUnreached();
+Function.prototype.apply = makeUnreached();
+Function.prototype.bind = makeUnreached();
+Function.prototype.call = makeUnreached();
+Array.from = makeUnreached();
+Array.isArray = makeUnreached();
+Array.prototype.concat = makeUnreached();
+Array.prototype.filter = makeUnreached();
+Array.prototype.forEach = makeUnreached();
+Array.prototype.indexOf = makeUnreached();
+Array.prototype.join = makeUnreached();
+Array.prototype.map = makeUnreached();
+Array.prototype.pop = makeUnreached();
+Array.prototype.push = makeUnreached();
+Array.prototype.reverse = makeUnreached();
+Array.prototype.shift = makeUnreached();
+Array.prototype.slice = makeUnreached();
+Array.prototype.splice = makeUnreached();
+Array.prototype.unshift = makeUnreached();
+String.prototype.indexOf = makeUnreached();
+String.prototype.replace = makeUnreached();
+String.prototype.slice = makeUnreached();
+String.prototype.split = makeUnreached();
+String.prototype.substr = makeUnreached();
+String.prototype.toLowerCase = makeUnreached();
+String.prototype.toUpperCase = makeUnreached();
+
+CustomElementRegistry.prototype.define = makeUnreached();
+Document.prototype.createElement = makeUnreached();
+Document.prototype.createEvent = makeUnreached();
+Element.prototype.attachShadow = makeUnreached();
+Element.prototype.getAttribute = makeUnreached();
+Element.prototype.getBoundingClientRect = makeUnreached();
+Element.prototype.hasAttribute = makeUnreached();
+Element.prototype.removeAttribute = makeUnreached();
+Element.prototype.setAttribute = makeUnreached();
+EventTarget.prototype.addEventListener = makeUnreached();
+EventTarget.prototype.dispatchEvent = makeUnreached();
+EventTarget.prototype.removeEventListener = makeUnreached();
+HTMLElement.prototype.focus = makeUnreached();
+MutationObserver.prototype.observe = makeUnreached();
+MutationObserver.prototype.takeRecords = makeUnreached();
+Node.prototype.appendChild = makeUnreached();
+Node.prototype.removeChild = makeUnreached();
+Node.prototype.replaceChild = makeUnreached();
+
+getComputedStyle = makeUnreached();
+parseInt = makeUnreached();
+parseFloat = makeUnreached();
+
+// Also overwrite constructors.
+MutationObserver = makeUnreached();
+Object = makeUnreached();
+Function = makeUnreached();
+Array = makeUnreached();
+String = makeUnreached();
+
+var tests = {
+  testCreate: () => {
+    var webview = new WebView();
+    webview.src = 'data:text/html,<body>Guest</body>';
+    webview.savedAddEventListener('loadstop', chrome.test.callbackPass());
+    document.body.savedAppendChild(webview);
+  },
+
+  testSetOnEventProperty: () => {
+    var webview = new WebView();
+    // Set and overwrite an on<event> property on the view.
+    webview.onloadstop = () => {};
+    webview.onloadstop = () => {};
+    chrome.test.succeed();
+  },
+
+  testGetSetAttributes: () => {
+    var webview = new WebView();
+
+    // Get and set various attribute types.
+    var url = 'data:text/html,<body>Guest</body>';
+    webview.src = url;
+    chrome.test.assertEq(url, webview.src);
+
+    webview.autosize = true;
+    chrome.test.assertTrue(webview.autosize);
+    webview.autosize = false;
+    chrome.test.assertFalse(webview.autosize);
+
+    webview.maxheight = 123;
+    chrome.test.assertEq(123, webview.maxheight);
+    webview.maxheight = undefined;
+    chrome.test.assertEq(0, webview.maxheight);
+
+    var name = 'my-webview';
+    webview.name = name;
+    chrome.test.assertEq(name, webview.name);
+    webview.name = undefined;
+    chrome.test.assertEq('', webview.name);
+
+    chrome.test.succeed();
+  },
+
+  testBackForward: () => {
+    var webview = new WebView();
+    // The back and forward methods are implemented in terms of go. Make sure
+    // they don't call an overwritten version.
+    webview.go = makeUnreached();
+    webview.back();
+    webview.forward();
+    chrome.test.succeed();
+  },
+
+  testFocus: () => {
+    var webview = new WebView();
+    webview.src = 'data:text/html,<body>Guest</body>';
+    webview.savedAddEventListener('loadstop', chrome.test.callbackPass(() => {
+      webview.focus();
+    }));
+    document.body.savedAppendChild(webview);
+  },
+};
+
+window.runTest = (testName) => {
+  if (!tests[testName]) {
+    chrome.test.notifyFail('Test does not exist: ' + testName);
+    return;
+  }
+
+  chrome.test.runTests([tests[testName]]);
+};

extensions/test/data/web_view/no_internal_calls_to_user_code/manifest.json

+{
+  "name": "<webview> use in the presence of methods overwritten by user code.",
+  "manifest_version": 2,
+  "version": "1",
+  "permissions": [
+    "webview"
+  ],
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}

extensions/test/data/web_view/no_internal_calls_to_user_code/test.js

+// Copyright 2018 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.
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  chrome.app.window.create('main.html', {}, function() {});
+});