[Extensions] Update web API cloberring for platform apps

extensions/renderer/resources/platform_app.js

Index: extensions/renderer/resources/platform_app.js
diff --git a/extensions/renderer/resources/platform_app.js b/extensions/renderer/resources/platform_app.js
index 21263d3d0c213e35f3b774feb362e5a0b8fcd711..4c7e067a5a368118243056f583198a709b555603 100644
--- a/extensions/renderer/resources/platform_app.js
+++ b/extensions/renderer/resources/platform_app.js
@@ -2,6 +2,8 @@
 // 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.
@@ -65,10 +67,16 @@ function generateThrowingMethodStub(messagePrefix, opt_messageSuffix) {
  */
 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)
+    });
   });
 }
 
@@ -85,9 +93,16 @@ function disableMethods(object, objectName, methodNames, useThrowingStubs) {
  *     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;
@@ -130,10 +145,12 @@ function disableGetters(object, objectName, propertyNames, opt_messageSuffix) {
  */
 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;
@@ -144,24 +161,26 @@ function disableSetters(object, objectName, propertyNames, opt_messageSuffix) {
 }
 
 // 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',
@@ -199,11 +218,14 @@ window.addEventListener('readystatechange', function(event) {
 
 // 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);
 };