Issue 4494 - Avoid causing some sandbox related warnings

inject.preload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2017 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 369 matching lines @@
   RealRTCPeerConnection.prototype.constructor = boundWrappedRTCPeerConnection;
 
   if ("RTCPeerConnection" in window)
     window.RTCPeerConnection = boundWrappedRTCPeerConnection;
   if ("webkitRTCPeerConnection" in window)
     window.webkitRTCPeerConnection = boundWrappedRTCPeerConnection;
 }
 
 if (document instanceof HTMLDocument)
 {
-  let script = document.createElement("script");
-  script.type = "application/javascript";
-  script.async = false;
-  script.textContent = "(" + injected + ")('" + randomEventName + "');";
-  document.documentElement.appendChild(script);
-  document.documentElement.removeChild(script);
+  let sandbox = window.frameElement &&
+                  window.frameElement.getAttribute("sandbox");

[Sebastian Noack, 2017/05/30 09:41:16]

Nit: The indentation looks a bit off here.

[kzar, 2017/05/30 10:20:31]

Done.

+  if (typeof sandbox != "string" || sandbox.includes("allow-scripts"))

[Sebastian Noack, 2017/05/30 09:41:16]

What if allow-scripts is misspelled like "allow-scriptss"?

[kzar, 2017/05/30 10:20:31]

Well a false-positive doesn't matter here, it just results in the warning being
displayed.

[Sebastian Noack, 2017/05/30 10:36:46]

I wonder whether we even need this check. If scripts are not allowed this code
won't run anyway, right?

[Sebastian Noack, 2017/05/30 10:43:34]

Never mind. We are in the extension's content script here, I mistakenly assumed
that this is part of the injected code.

Anyway, what I was initially suggesting here, was to use
/(^|\s)allow-scripts($|\s)/.test(sandbox), which would be a little more
accurrate. But it's not too important. I leave it up to you. LGTM either way.

[kzar, 2017/05/30 12:27:15]

Doing some further reading I realised that
window.frameElement.sandbox.contains("allow-scripts") would be even better. But
then I realised DOMTokenList is case sensitive, so while "ALLOW-SCRIPTS" would
work, we wouldn't detect it! This was really bad, giving websites a way to
circumvent our wrappers. So instead I've gone with your approach, adding the
case insensitive flag. Done.

+  {
+    let script = document.createElement("script");
+    script.type = "application/javascript";
+    script.async = false;
+    script.textContent = "(" + injected + ")('" + randomEventName + "');";
+    document.documentElement.appendChild(script);
+    document.documentElement.removeChild(script);
+  }
 }