Issue 1727 - Prevent circumvention via WebSocket

include.preload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 337 matching lines @@
 
   observer.observe(parentNode, {childList: true});
   return observer;
 }
 
 function runInPage(fn, arg)
 {
   var script = document.createElement("script");
   script.type = "application/javascript";
   script.async = false;
-
-  // include.youtube.js passes this function is passed a RegExp which

[Sebastian Noack, 2016/08/09 15:09:37]

This reads weird. Typo?

  In include.youtube.js, this function is passed a RegExp ...

[kzar, 2016/08/09 16:11:40]

Done.

-  // JSON.stringify would convert to "{}".
-  if (!(arg instanceof RegExp))

[Sebastian Noack, 2016/08/09 14:53:30]

It seems this wasn't necessary before. So why is it now?

[Sebastian Noack, 2016/08/09 15:09:37]

Never mind, I got it, (misinterpreted the condition before).

However, note that alternatively, we could move JSON.stringify() into the
calling code, avoiding a special case here. Not sure, which is better though..

-    arg = JSON.stringify(arg);
-
-  script.textContent = "(" + fn + ")(" + arg + ");";
+  script.textContent = "(" + fn + ")(" + JSON.stringify(arg) + ");";
   document.documentElement.appendChild(script);
   document.documentElement.removeChild(script);
 }
 
 function protectStyleSheet(document, style)
 {
   style.id = id;
 
-  var protector = function(id)
+  runInPage(function(id)
   {
     var style = document.getElementById(id) ||
                 document.documentElement.shadowRoot.getElementById(id);
     style.removeAttribute("id");
 
     var disableables = [style, style.sheet];
     for (var i = 0; i < disableables.length; i++)
       Object.defineProperty(disableables[i], "disabled",
                             {value: false, enumerable: true});
 
     ["deleteRule", "removeRule"].forEach(function(method)
     {
       var original = CSSStyleSheet.prototype[method];
       CSSStyleSheet.prototype[method] = function(index)
       {
         if (this != style.sheet)
           original.call(this, index);
       };

[Sebastian Noack, 2016/08/09 15:41:15]

Bind those functions as well? Otherwise we can still be detect.

[kzar, 2016/08/09 16:11:40]

Done.

     });
-  };
-
-  runInPage(protector, id);
+  }, id);
 }
 
 // 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, preventing blocked WebSocket
 // connections from being opened.
 // [1] - https://bugs.chromium.org/p/chromium/issues/detail?id=129353
 function wrapWebSocket()
 {
   if (typeof WebSocket == "undefined")
     return;
 
   var eventName = "abpws-" + id;
 
   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)

[Sebastian Noack, 2016/08/09 14:53:30]

I just noticed that the argument name is the same as the name of the variable in
the outer closure. That might be confusing, potentially raising wrong
assumptions when reading the code. While they are essentially the same, it needs
to be explicitly passed in the generated code.

[Sebastian Noack, 2016/08/09 14:53:30]

I just noticed a minor inconsistency, while you use an anonymous function above
(as well ass in include.youtube.js), you pass a named function to run
runInPage() here, which results into following code to be injected:

  (function wrapper(eventName)
  {
    ...
  })(...);

instead of


  (function (eventName)
  {
    ...
  })(...);

FWIW, I'd prefer the latter, as it keeps the generated/injected code simpler and
shorter. Also I'm not sure if using a named function could potentially leak the
function in any JavaScript version/implementation, doesn't seem to be the case
in modern Chrome versions though.

(Or perhaps, skip the variable all together, and pass an anonymouse function
directly to runInPage().)

[kzar, 2016/08/09 16:11:39]

Done.

[kzar, 2016/08/09 16:11:40]

I'd rather leave this being called eventName, I think especially now that the
function's not named it's pretty obvious what's going on. (If you insist I
suppose I don't mind changing it, but in that case do you have a suggestion for
a better name?)

+  runInPage(function(eventName)
   {
     // As far as possible we must track everything we use that could be
     // sabotaged by the website later in order to circumvent us.
     var RealWebSocket = WebSocket;
-    var closeWebSocket = RealWebSocket.prototype.close;
+    var closeWebSocket = Function.prototype.call.bind(RealWebSocket.prototype.close);
     var addEventListener = document.addEventListener.bind(document);
     var removeEventListener = document.removeEventListener.bind(document);
     var dispatchEvent = document.dispatchEvent.bind(document);
     var CustomEvent = window.CustomEvent;
-    var boundCall = Function.prototype.call.bind(Function.prototype.call);
-    var functionToString = Function.prototype.toString;
-    // (Safari 9 considers WebSocket to be an object rather than a function.)
-    var webSocketString = RealWebSocket.toString();
 
     function checkRequest(url, callback)
     {
       var incomingEventName = eventName + "-" + url;
       function listener(event)
       {
         callback(event.detail);
         removeEventListener(incomingEventName, listener);
       }
       addEventListener(incomingEventName, listener);
 
       dispatchEvent(new CustomEvent(eventName, {
         detail: {url: url}
       }));
     }
 
-    function wrappedToString()
-    {
-      if (this === WebSocket)
-        return webSocketString;
-      if (this === wrappedToString)
-        return boundCall(functionToString, functionToString);
-      return boundCall(functionToString, this);
-    };
-    Function.prototype.toString = wrappedToString;
-
-    WebSocket = function(url, protocols)
-    {
+    WebSocket = function WrappedWebSocket(url, protocols)
+    {
+      // Throw correct exceptions if the constructor is used improperly.
+      if (!(this instanceof WrappedWebSocket)) return RealWebSocket();
+      if (arguments.length < 1) return new RealWebSocket();
+
       var websocket = new RealWebSocket(url, protocols);
 
       checkRequest(websocket.url, function(blocked)
       {
         if (blocked)
-          boundCall(closeWebSocket, websocket);
+          closeWebSocket(websocket);
       });
 
       return websocket;
-    };
-
-    var properties = Object.getOwnPropertyNames(RealWebSocket);
-    for (var i = 0; i < properties.length; i++)
-    {
-      var name = properties[i];
-      var desc = Object.getOwnPropertyDescriptor(RealWebSocket, name);
-      Object.defineProperty(WebSocket, name, desc);
-    }
+    }.bind();
+
+    Object.defineProperties(WebSocket, {
+      CONNECTING: {value: RealWebSocket.CONNECTING, enumerable: true},
+      OPEN: {value: RealWebSocket.OPEN, enumerable: true},
+      CLOSING: {value: RealWebSocket.CLOSING, enumerable: true},
+      CLOSED: {value: RealWebSocket.CLOSED, enumerable: true},
+      prototype: {value: RealWebSocket.prototype}
+    });
 
     RealWebSocket.prototype.constructor = WebSocket;
-  }
-
-  runInPage(wrapper, eventName);
+  }, eventName);
 }
 
 function init(document)
 {
   var shadow = null;
   var style = null;
   var observer = null;
   var tracer = null;
 
   wrapWebSocket();
@@ skipping 161 matching lines @@
   }, true);
 
   return updateStylesheet;
 }
 
 if (document instanceof HTMLDocument)
 {
   checkSitekey();
   window.updateStylesheet = init(document);
 }