Issue 5382 - Wrap DOM mutation APIs to protect frames

inject.preload.js

Index: inject.preload.js
===================================================================
--- a/inject.preload.js
+++ b/inject.preload.js
@@ -67,53 +67,52 @@
           "(" + injectedToString() + ")('" + eventName + "', true);"
         );
         delete contentWindow[eventName];
       }
       catch (e) {}
     }
   }
 
-  for (let element of [HTMLFrameElement, HTMLIFrameElement, HTMLObjectElement])
+  function injectIntoAllFrames()
+  {
+    for (let i = 0; i < window.length; i++)
+      injectIntoContentWindow(window[i]);
+  }
+
+  function wrapAPIForInjection(object, api, callback)
   {
-    let contentDocumentDesc = Object.getOwnPropertyDescriptor(
-      element.prototype, "contentDocument"
-    );
-    let contentWindowDesc = Object.getOwnPropertyDescriptor(
-      element.prototype, "contentWindow"
-    );
-
-    // Apparently in HTMLObjectElement.prototype.contentWindow does not exist
-    // in older versions of Chrome such as 42.
-    if (!contentWindowDesc)
-      continue;
+    let func = object[api];
+    object[api] = function(...args)
+    {
+      let returnValue = func.apply(this, args);

[kzar, 2017/12/11 08:48:13]

I think func.apply could have been messed with at this point. I recommend
reading through all Tartpvule's comments[1] on the similar codereviews to get an
idea about some of the things we have to watch out for. There's likely some
other similar problems.

[1] -
https://codereview.adblockplus.org/search?closed=1&owner=&reviewer=chinrtgm1%...

[Manish Jethani, 2018/02/21 16:37:39]

Thanks, for this one in particular now I've bound the function to
Function.prototype.apply similar to what we're doing in other places.

+      callback(returnValue);
+      return returnValue;
+    };
+  }
 
-    let getContentDocument = Function.prototype.call.bind(
-      contentDocumentDesc.get
-    );
-    let getContentWindow = Function.prototype.call.bind(
-      contentWindowDesc.get
-    );
+  function wrapPropertyAPIForInjection(object, api, method, callback)
+  {
+    let descriptor = Object.getOwnPropertyDescriptor(object, api);
+    wrapAPIForInjection(descriptor, method, callback);
+    Object.defineProperty(object, api, descriptor);
+  }
 
-    contentWindowDesc.get = function()
-    {
-      let contentWindow = getContentWindow(this);
-      injectIntoContentWindow(contentWindow);
-      return contentWindow;
-    };
-    contentDocumentDesc.get = function()
-    {
-      injectIntoContentWindow(getContentWindow(this));
-      return getContentDocument(this);
-    };
-    Object.defineProperty(element.prototype, "contentWindow",
-                          contentWindowDesc);
-    Object.defineProperty(element.prototype, "contentDocument",
-                          contentDocumentDesc);
-  }
+  wrapAPIForInjection(Node.prototype, "appendChild", injectIntoAllFrames);
+  wrapAPIForInjection(Node.prototype, "insertBefore", injectIntoAllFrames);
+  wrapAPIForInjection(Node.prototype, "replaceChild", injectIntoAllFrames);
+
+  wrapPropertyAPIForInjection(Element.prototype,
+                              "innerHTML", "set", injectIntoAllFrames);
+
+  wrapPropertyAPIForInjection(HTMLObjectElement.prototype,

[Manish Jethani, 2017/10/24 01:43:28]

HTMLFrameElement and HTMLIFrameElement don't need wrappers now because we're
injecting the code preemptively.

For HTMLObjectElement we don't have to worry about Chrome 42, it's too old now.

[kzar, 2017/12/11 08:48:13]

Fair enough but did you check that it's OK with Chrome 49?

[Manish Jethani, 2018/02/21 16:37:39]

I've added a check now so if the descriptor is not found it'll just return.

+                              "contentWindow", "get", injectIntoContentWindow);
+  wrapPropertyAPIForInjection(HTMLObjectElement.prototype,
+                              "contentDocument", "get",
+                              doc => injectIntoContentWindow(doc.defaultView));
 
   /*
    * Shadow root getter wrapper
    *
    * After creating our shadowRoot we must wrap the getter to prevent the
    * website from accessing it (#4191, #4298). This is required as a
    * workaround for the lack of user style support in Chrome.
    * See https://bugs.chromium.org/p/chromium/issues/detail?id=632009&desc=2