Issue 1727 - Prevent circumvention via WebSocket

include.preload.js

Index: include.preload.js
diff --git a/include.preload.js b/include.preload.js
index 949d039663f9130843b4d8d0a3ddfcb434522066..af018d16fde5651abfea0f3241cb8658493479cb 100644
--- a/include.preload.js
+++ b/include.preload.js
@@ -349,9 +349,9 @@ function reinjectStyleSheetWhenRemoved(document, style)
   return observer;
 }
 
-function protectStyleSheet(document, style)
+function protectStyleSheet(document, style, script)
 {
-  var id = Math.random().toString(36).substr(2)
+  var id = Math.random().toString(36).substr(2);
   style.id = id;
 
   var code = [
@@ -387,11 +387,155 @@ function protectStyleSheet(document, style)
 
   code.push("})();");
 
-  var script = document.createElement("script");
-  script.async = false;
-  script.textContent = code.join("\n");
-  document.documentElement.appendChild(script);
-  document.documentElement.removeChild(script);
+  script.textContent += code.join("\n");
+}
+
+// Neither Chrome[1] nor Safari allow us to intercept WebSockets, and therefore
+// some ad networks are misusing them as a way to serve adverts and circumvent
+// us. As a workaround we wrap WebSocket, closing connections that would have
+// otherwise been blocked.
+// [1] - https://bugs.chromium.org/p/chromium/issues/detail?id=129353
+function wrapWebSocket(script)
+{
+  if (typeof WebSocket == "undefined")
+    return;
+
+  var eventName = "abpws-" + Math.random().toString().substr(2);

[Sebastian Noack, 2016/07/01 17:03:39]

Instead of duplicating this logic, perhaps just assign the random string to a
global variable? It doesn't matter if it's the same.

[kzar, 2016/07/06 16:39:19]

Done.

+
+  document.addEventListener(eventName, function(event)
+  {
+    ext.backgroundPage.sendMessage({
+      type: "websocket-request",
+      url: event.detail.url
+    }, function (block)
+    {
+      document.dispatchEvent(
+        new CustomEvent(eventName + "-" + event.detail.url, {detail: block})
+      );
+    });
+  });
+
+  function wrapper(eventName)
+  {
+    var originalWebSocket = WebSocket;
+    var readyStates = {
+      CLOSED: {value: 3, enumerable: true},
+      CLOSING: {value: 2, enumerable: true},
+      OPEN: {value: 1, enumerable: true},
+      CONNECTING: {value: 0, enumerable: true}
+    };
+
+    WebSocket = function(url, protocol)
+    {
+      var blocked = false;
+      var websocket;

[Sebastian Noack, 2016/07/01 17:03:39]

Since you access this variable before it might get set, please initialize with
null.

[kzar, 2016/07/06 16:39:19]

Done.

+      var websocketProperties = {

[Sebastian Noack, 2016/07/01 17:03:39]

It seems you don't need to hard-code that properties but can get the list by
Object.getOwnPropertyNames(WebSocket.prototype), though obviously not the
default values without actually creating a WebSocket.

[kzar, 2016/07/06 16:39:19]

Done.

+        attributes: {
+          url: url,
+          protocol: protocol,
+          readyState: readyStates.CONNECTING,
+          bufferedAmount: 0,
+          extensions: "",
+          binaryType: "blob",
+          onopen: null,
+          onerror: null,
+          onclose: null,
+          onmessage: null
+        },
+        methods: ["close", "send", "addEventListener", "removeEventListener"]
+      };
+      var properties = Object.create(null);
+      var queue = [];
+
+      for (var key in websocketProperties.attributes)
+      {
+        properties[key] = function(key, defaultValue)
+        {
+          return {
+            enumerable: true,

[Sebastian Noack, 2016/07/01 17:03:40]

Instead hard-coding metadata like "enumerable" you could get them from the
original descriptor to ensure consistent behavior:

  Object.getOwnPropertyDescriptor(WebSocket.prototype, key)

[kzar, 2016/07/06 16:39:19]

Done.

+            get: function()
+            {
+              if (blocked && key == "readyState")
+                return readyStates.CLOSED;
+              return websocket ? websocket[key] : defaultValue;
+            },
+            set: function(value)
+            {
+              if (websocket)
+                return websocket[key] = value;

[Sebastian Noack, 2016/07/01 17:03:40]

Why do you return in a setter?

[kzar, 2016/07/06 16:39:20]

Because the return value for an assignment is usually the value assigned.

[Sebastian Noack, 2016/08/08 16:42:53]

An assignment operation returns it's right-hand value regardless of what the
setter returns.

[kzar, 2016/08/08 18:19:05]

Ah, I didn't know that!

+              else
+              {
+                queue.push(["set", key, value]);
+                return value;
+              }
+            }
+          };
+        }(key, websocketProperties.attributes[key]);
+      }
+      for (var i = 0; i < websocketProperties.methods.length; i += 1)
+      {
+        var method = websocketProperties.methods[i];
+        properties[method] = (function(method)
+        {
+          return {
+            enumerable: true,
+            value: function()
+            {
+              if (websocket)
+                websocket[method].apply(websocket, arguments);
+              else
+                queue.push(["call", method, arguments]);
+            }
+          };
+        }(method));
+      }
+      Object.defineProperties(this, properties);

[Sebastian Noack, 2016/07/01 17:03:40]

Note that those properties (in Chrome at least) are not on the WebSocket object
but on it's prototype. So please consider doing the same, to avoid side effects.
At the very least websites could easily detect that hack otherwise.

[kzar, 2016/07/06 16:39:20]

Done.

+
+      function processQueue()
+      {
+        for (var i = 0; i < queue.length; i += 1)
+        {
+          var action = queue[i][0];
+          var key = queue[i][1];
+          var value = queue[i][2];
+          switch (action)
+          {
+            case "set":
+              websocket[key] = value;
+              break;
+            case "call":
+              websocket[key].apply(websocket, value);
+              break;
+          }
+        }
+        queue = undefined;
+      }
+
+      var incomingEventName = eventName + "-" + url;
+      function listener(event)
+      {
+        blocked = event.detail;
+        if (blocked)
+          queue = undefined;
+        else
+        {
+          websocket = new originalWebSocket(url, protocol);
+          processQueue();
+        }
+
+        document.removeEventListener(incomingEventName, listener);
+      }
+      document.addEventListener(incomingEventName, listener);
+
+      document.dispatchEvent(new CustomEvent(eventName, {
+        detail: {url: url, protocol: protocol}
+      }));
+    };
+    WebSocket.prototype = Object.create(window.EventTarget.prototype, readyStates);

[Sebastian Noack, 2016/07/01 17:03:40]

Can't you simply reuse the original parent prototype?

  WebSocket.prototype = Object.getPrototypeOf(originalWebSocket.prototype)

[kzar, 2016/07/06 16:39:20]

Done. (Good point as it is different in Safari.)

+    Object.defineProperties(WebSocket, readyStates);
+  }
+
+  script.textContent += "(" + wrapper.toString() + ")(\"" + eventName + "\");";

[Sebastian Noack, 2016/07/01 17:03:40]

This is inconsistent with how we generate the code to prevent messing with the
stylesheet where we use plain string concatenation, while we convert a function
to a string here. I don't mind either way, but would you mind to keep it
consistent?

[kzar, 2016/07/06 16:39:19]

Done, however wasn't sure how to test the code above still works.

 }
 
 function init(document)
@@ -401,6 +545,12 @@ function init(document)
   var observer = null;
   var tracer = null;
 
+  // Scripts to be injected into the page, executed at the end of initialization
+  var script = document.createElement("script");
+  script.async = false;
+
+  wrapWebSocket(script);
+
   function getPropertyFilters(callback)
   {
     ext.backgroundPage.sendMessage({
@@ -448,7 +598,8 @@ function init(document)
         return;
 
       observer = reinjectStyleSheetWhenRemoved(document, style);
-      protectStyleSheet(document, style);
+      if (script)

[Sebastian Noack, 2016/07/01 17:03:39]

Sorry, I didn't notice that this a different code path. So I guess it actually
makes more sense to inject the scripts separately. Besides, the worse code
locality this change prevents the stylesheet to be protected if it's updated
later.

[kzar, 2016/07/06 16:39:19]

Done.

+        protectStyleSheet(document, style, script);
     }
 
     // If using shadow DOM, we have to add the ::content pseudo-element
@@ -563,6 +714,10 @@ function init(document)
     }
   }, true);
 
+  document.documentElement.appendChild(script);
+  document.documentElement.removeChild(script);
+  script = undefined;
+
   return updateStylesheet;
 }