Issue 5382 - Wrap DOM mutation APIs to protect frames

inject.preload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 54 matching lines @@
         contentWindow[eventName] = checkRequest;
         contentWindow.eval(
           "(" + injectedToString() + ")('" + eventName + "', true);"
         );
         delete contentWindow[eventName];
       }
       catch (e) {}
     }
   }
 
-  for (let element of [HTMLFrameElement, HTMLIFrameElement, HTMLObjectElement])
+  function injectIntoAllFrames()
   {
-    let contentDocumentDesc = Object.getOwnPropertyDescriptor(
-      element.prototype, "contentDocument"
-    );
-    let contentWindowDesc = Object.getOwnPropertyDescriptor(
-      element.prototype, "contentWindow"
-    );
+    for (let i = 0; i < window.length; i++)
+      injectIntoContentWindow(window[i]);
+  }
 
-    // Apparently in HTMLObjectElement.prototype.contentWindow does not exist
-    // in older versions of Chrome such as 42.
-    if (!contentWindowDesc)
-      continue;
+  function wrapAPIForInjection(object, api, callback)
+  {
+    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
    */
   if ("shadowRoot" in Element.prototype)
@@ skipping 274 matching lines @@
   if (typeof sandbox != "string" || /(^|\s)allow-scripts(\s|$)/i.test(sandbox))
   {
     let script = document.createElement("script");
     script.type = "application/javascript";
     script.async = false;
     script.textContent = "(" + injected + ")('" + randomEventName + "');";
     document.documentElement.appendChild(script);
     document.documentElement.removeChild(script);
   }
 }