[Extensions] Update web API cloberring for platform apps

extensions/renderer/resources/platform_app.js

 // Copyright 2014 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 logging = requireNative('logging');
+
 /**
  * Returns a function that logs a 'not available' error to the console and
  * returns undefined.
  *
  * @param {string} messagePrefix text to prepend to the exception message.
  */
 function generateDisabledMethodStub(messagePrefix, opt_messageSuffix) {
   var message = messagePrefix + ' is not available in packaged apps.';
   if (opt_messageSuffix) message = message + ' ' + opt_messageSuffix;
   return function() {
@@ skipping 43 matching lines @@
  *     preferred.
  * @param {string} objectName The display name to use in the error message
  *     thrown by the stub (this is the name that the object is commonly referred
  *     to by web developers, e.g. "document" instead of "HTMLDocument").
  * @param {Array<string>} methodNames names of methods to disable.
  * @param {Boolean} useThrowingStubs if true, the replaced methods will throw
  *     an error instead of silently returning undefined
  */
 function disableMethods(object, objectName, methodNames, useThrowingStubs) {
   $Array.forEach(methodNames, function(methodName) {
+    logging.DCHECK($Object.getOwnPropertyDescriptor(object, methodName),
+                   objectName + ': ' + methodName);
     var messagePrefix = objectName + '.' + methodName + '()';
-    object[methodName] = useThrowingStubs ?
-        generateThrowingMethodStub(messagePrefix) :
-        generateDisabledMethodStub(messagePrefix);
+    $Object.defineProperty(object, methodName, {
+      configurable: false,
+      enumerable: false,
+      value: useThrowingStubs ?
+                 generateThrowingMethodStub(messagePrefix) :
+                 generateDisabledMethodStub(messagePrefix)
+    });
   });
 }
 
 /**
  * Replaces the given properties of the passed in object with stubs that log
  * 'not available' warnings to the console and return undefined when gotten. If
  * a property's setter is later invoked, the getter and setter are restored to
  * default behaviors.
  *
  * @param {Object} object The object with properties to disable. The prototype
  *     is preferred.
  * @param {string} objectName The display name to use in the error message
  *     thrown by the getter stub (this is the name that the object is commonly
  *     referred to by web developers, e.g. "document" instead of
  *     "HTMLDocument").
  * @param {Array<string>} propertyNames names of properties to disable.
+ * @param {?string=} opt_messageSuffix An optional suffix for the message.
+ * @param {boolean=} opt_ignoreMissingProperty True if we allow disabling
+ *     getters for non-existent properties.
  */
-function disableGetters(object, objectName, propertyNames, opt_messageSuffix) {
+function disableGetters(object, objectName, propertyNames, opt_messageSuffix,
+                        opt_ignoreMissingProperty) {
   $Array.forEach(propertyNames, function(propertyName) {
+    logging.DCHECK(opt_ignoreMissingProperty ||
+                       $Object.getOwnPropertyDescriptor(object, propertyName),
+                   objectName + ': ' + propertyName);
     var stub = generateDisabledMethodStub(objectName + '.' + propertyName,
                                           opt_messageSuffix);
     stub._is_platform_app_disabled_getter = true;
     $Object.defineProperty(object, propertyName, {
       configurable: true,
       enumerable: false,
       get: stub,
       set: function(value) {
         var descriptor = $Object.getOwnPropertyDescriptor(this, propertyName);
         if (!descriptor || !descriptor.get ||
@@ skipping 22 matching lines @@
  * @param {Object} object The object with properties to disable. The prototype
  *     is preferred.
  * @param {string} objectName The display name to use in the error message
  *     thrown by the setter stub (this is the name that the object is commonly
  *     referred to by web developers, e.g. "document" instead of
  *     "HTMLDocument").
  * @param {Array<string>} propertyNames names of properties to disable.
  */
 function disableSetters(object, objectName, propertyNames, opt_messageSuffix) {
   $Array.forEach(propertyNames, function(propertyName) {
+    logging.DCHECK($Object.getOwnPropertyDescriptor(object, propertyName),
+                   objectName + ': ' + propertyName);
     var stub = generateDisabledMethodStub(objectName + '.' + propertyName,
                                           opt_messageSuffix);
     $Object.defineProperty(object, propertyName, {
-      configurable: true,
+      configurable: false,
       enumerable: false,
       get: function() {
         return;
       },
       set: stub
     });
   });
 }
 
 // Disable benign Document methods.
-disableMethods(HTMLDocument.prototype, 'document', ['open', 'clear', 'close']);
+disableMethods(Document.prototype, 'document', ['open', 'close']);
+disableMethods(HTMLDocument.prototype, 'document', ['clear']);
 
 // Replace evil Document methods with exception-throwing stubs.
-disableMethods(HTMLDocument.prototype, 'document', ['write', 'writeln'], true);
+disableMethods(Document.prototype, 'document', ['write', 'writeln'], true);
 
 // Disable history.
 Object.defineProperty(window, "history", { value: {} });
+// Note: we just blew away the history object, so we need to ignore the fact
+// that these properties aren't defined on the object.
 disableGetters(window.history, 'history',
-    ['back', 'forward', 'go', 'length', 'pushState', 'replaceState', 'state']);
+    ['back', 'forward', 'go', 'length', 'pushState', 'replaceState', 'state'],
+    null, true);
 
 // Disable find.
 disableMethods(window, 'window', ['find']);
-disableMethods(Window.prototype, 'window', ['find']);
 
 // Disable modal dialogs. Shell windows disable these anyway, but it's nice to
 // warn.
 disableMethods(window, 'window', ['alert', 'confirm', 'prompt']);
-disableMethods(Window.prototype, 'window', ['alert', 'confirm', 'prompt']);
 
 // Disable window.*bar.
 disableGetters(window, 'window',
     ['locationbar', 'menubar', 'personalbar', 'scrollbars', 'statusbar',
      'toolbar']);
 
 // Disable window.localStorage.
 // Sometimes DOM security policy prevents us from doing this (e.g. for data:
 // URLs) so wrap in try-catch.
 try {
@@ skipping 17 matching lines @@
   // To deprecate document.all, simply changing its getter and setter would
   // activate its cache mechanism, and degrade the performance. Here we assign
   // it first to 'undefined' to avoid this.
   document.all = undefined;
   disableGetters(document, 'document',
       ['alinkColor', 'all', 'bgColor', 'fgColor', 'linkColor', 'vlinkColor']);
 }, true);
 
 // Disable onunload, onbeforeunload.
 disableSetters(window, 'window', ['onbeforeunload', 'onunload']);
-disableSetters(Window.prototype, 'window', ['onbeforeunload', 'onunload']);
 var eventTargetAddEventListener = EventTarget.prototype.addEventListener;
-EventTarget.prototype.addEventListener = function(type) {
+EventTarget.prototype.addEventListener = function() {
+  var args = $Array.slice(arguments);
+  // Note: Force conversion to a string in order to catch any funny attempts
+  // to pass in something that evals to 'load' but wouldn't === 'load'.

[robwu, 2016/02/19 22:32:15]

Nit: 'load' -> 'onload'

[Devlin, 2016/02/19 23:23:11]

s/onload/unload Done.

+  var type = args[0] + '';

[robwu, 2016/02/19 22:32:15]

Sorry, this is still not sufficient.

You have to *change* args[0] to a primitive string (the += in my suggestion was
very significant). The check here can still be bypassed with the definition of
type from my 1st review comment.

// Example:
var myType = {
  toString() {
    myType.toString = function() { return 'beforeunload'; };
    return 'xxx';
  }
};
var myFunc = function() { return "shouldn't happen"; }
addEventListener(myType, myFunc);

// This happens:
var args = $Array.slice(arguments);
// args = [myType, myFunc]

var type = args[0] + '';
// is similar to type = myType.toString() + ''; // = 'xxx'
// and args[0] = {toString: function() { return 'beforeunload'; }}.

if (type === 'unload' || type === 'beforeunload')
// Because type === 'xxx', both conditions are obviously not true

else
  return $Function.apply(eventTargetAddEventListener, this, args);
// addEventListener now converts args[0] to a string.
// args[0] is an object, args[0].toString() === 'beforeunload'
// ---> Bypassed!

[Devlin, 2016/02/19 23:23:11]

Ah, I missed a few of the subtleties when I looked at this earlier.  Very
clever.  Now addressed and tested.

   if (type === 'unload' || type === 'beforeunload')
     generateDisabledMethodStub(type)();
   else
-    return $Function.apply(eventTargetAddEventListener, this, arguments);
+    return $Function.apply(eventTargetAddEventListener, this, args);
 };