gorhill / uBO-Extra

A companion extension to uBlock Origin
GNU General Public License v3.0
654 stars 42 forks source link

Circumvention through event.target, event.srcElement and event.currentTarget #3

Closed kzar closed 8 years ago

kzar commented 8 years ago

I'm currently working on something similar for Adblock Plus and Lain noticed a weakness with our implementations. Event listeners for allowed WebSocket connections receive the real events, which include a way back to the real WebSocket.

I believe all the troublesome properties are target, srcElement and currentTarget.

Here's an example which gets rid of your WebSocket wrapper, restoring the real thing:

var ws = new WebSocket("ws://some-bogus-url");
ws.onerror = function(e) { WebSocket = e.target.__proto__.constructor; };

(I guess the solution is to manage the event listeners yourself, dispatching events with sensitive properties stripped. It is a pain though.)

Keep up the good work anyway,

Cheers, Dave.

gorhill commented 8 years ago

The e.target case one was solved in https://github.com/gorhill/uBlock/issues/1805 yesterday. However it did not occur to me to also cover srcElement and currentTarget entries, thanks for bringing that up.

But anyways there is a bigger issue with the wrapper in how it is easy to circumvent it, as also reported yesterday: https://github.com/gorhill/chromium-websocket-wrapper/issues/2. With this one, I don't think it's worth trying to push the websocket wrapper much further, ultimately the issue has to be fixed by Chromium devs.

The only solid solution I see given the last case is to not rely on a wrapper, but rather on injecting a CSP directive connect-src 'self' http: https: in the response headers, this can't be worked around by web sites. Unfortunately this removes the ability of the extension to report what a web page tried to do (though I did not experiment with tricking report-uri to report to the extension), and this also removes the leveraging of existing filters for websockets, but that will definitely prevents all attempts to connect to wss://, ws://.

kzar commented 8 years ago

Ah, sorry I didn't realise Lain already filed an issue about it. Thanks for other example of circumvention, I did not consider that either.

kzar commented 8 years ago

(Turns out the other example of circumvention wasn't a problem for Adblock Plus, so I figured out why. Here's a pull request https://github.com/gorhill/uBO-WebSocket/pull/4 for that.)

gorhill commented 8 years ago

Ok so https://github.com/gorhill/uBlock/issues/1805 also takes care of srcElement and currentTarget since these all refer back to the patched WebSocket instance.