Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adguard does not block WebSocket connection #203

Closed
ameshkov opened this issue Mar 20, 2016 · 16 comments
Closed

Adguard does not block WebSocket connection #203

ameshkov opened this issue Mar 20, 2016 · 16 comments
Assignees
Labels
Milestone

Comments

@ameshkov
Copy link
Member

ameshkov commented Mar 20, 2016

Refresh this page a couple of times:
http://seasonvar.ru/serial-13138-Ostrov_rus.html

Until you see this script there:


    function sendMessage(t){waitForSocketConnection(mg_ws,function(){mg_ws.send(t)})}function waitForSocketConnection(t,e){setTimeout(function(){return 1===t.readyState?void(null!=e&&e()):void waitForSocketConnection(t,e)},5)}var MarketGidDate=new Date,script=document.createElement("script"),mg_ws=new WebSocket("ws://wsp.marketgid.com:8040/ws");script.type="text/javascript",script.charset="utf-8",script.src="//jsc.marketgid.com/s/e/seasonvar.ru.583889.js?t=" + MarketGidDate.getYear() + MarketGidDate.getMonth() + MarketGidDate.getDay() + MarketGidDate.getHours(),script.onerror=function(){for(var t="MarketGidComposite583889",e=document.getElementById(t),n="",r="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789",o=0;25>o;o++)n+=r.charAt(Math.floor(Math.random()*r.length));var a=document.createElement("div");a.id=n,e.parentNode.insertBefore(a,e.nextSibling);e.parentNode.removeChild(e),mg_ws.onmessage=function(t){window.eval(t.data)},sendMessage("js|"+script.src+"|"+t+"|"+n),a.addEventListener("click",function(t){t.preventDefault();for(var e=t.target;"A"!=e.tagName&&e.id!=a;){if(e=e.parentNode,e.id==a)return;var n=e.href}return window.location=n,!1})},document.body.appendChild(script);

It seems that AG does not block WS connections (due to extension manifest I guess, we handle only http and https now).

Relevant discussion on our forum:
https://forum.adguard.com/index.php?threads/adguard-doesnt-block-adverts-on-newsru-com.9734/#post-79210

Websites to test WS is ok (got from this issue: gorhill/uBlock#1604)

I know of these two, they use websockets to retrieve images:
http://www.opensubtitles.org/en/search
http://thewatchseries.to/cale.html?r=aHR0cDovL2dvcmlsbGF2aWQuaW4veDBlM252eWk2MXh2
http://twitch.tv/twitch (example) is also affected. The IRC takes ages to load and disabling this extension completely(disabling it for a specfiic website does not) fixes it.
https://www.wunderlist.com/webapp for example ... even with the site whitelisted messing with the WebSocket class is breaking binding to websockets
http://www.pwnwin.com/dashboard - the header and sidebar of this site does not load

@ameshkov ameshkov added the Bug label Mar 20, 2016
@ameshkov ameshkov added this to the 2.3 milestone Mar 20, 2016
@ameshkov
Copy link
Member Author

ameshkov commented Apr 4, 2016

More information about web sockets in FF add-on:
http://stackoverflow.com/questions/11728871/firefox-extension-and-websocket-support?lq=1

Maybe these requests go through nsIContentPolicy, we should simply check it first.

@ameshkov
Copy link
Member Author

ameshkov commented Apr 5, 2016

Some information about Chrome:
http://stackoverflow.com/questions/22868897/access-websocket-traffic-from-chrome-extension

Quote from there:

Currently, the only way to access or modify Websocket traffic is to use a content script to inject a script that replaces the WebSocket constructor with your own wrapper. This wrapper should behave like the original WebSocket implementation, but you can add more stuff, like logging the sent/received messages to your extension.

@Alex-302
Copy link
Member

Alex-302 commented Apr 5, 2016

Example:
http://ikinohd.com/15767-betmen-protiv-supermena.html

![image](http://imagizer.imageshack.com/img924/4999/IAAYiF.png)
ws://wsp.marketgid.com:8040/ws

@Mizzick
Copy link
Contributor

Mizzick commented Apr 6, 2016

So it's not possible for now?

@ameshkov
Copy link
Member Author

ameshkov commented Apr 6, 2016

Why? My comment describes how it is possible.

@ameshkov
Copy link
Member Author

ameshkov commented Apr 7, 2016

Documentation:
https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API/Writing_WebSocket_client_applications

Some important notes on it

  1. If this connection is blocked we should emulate connection error:
    https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API/Writing_WebSocket_client_applications#Connection_errors

Other option: change URL in the original websocket to 127.0.0.1 so it will throw connection error naturally.

Mizzick added a commit that referenced this issue Apr 22, 2016
Mizzick added a commit that referenced this issue Apr 27, 2016
Mizzick added a commit that referenced this issue Apr 27, 2016
Mizzick added a commit that referenced this issue Apr 27, 2016
@gorhill
Copy link

gorhill commented May 3, 2016

@ameshkov I wrote such wrapper to expose WebSocket connection (ref), using as a standalone "companion" extension. I am considering making the extension more generic so as to not cater to one specific blocker, but to rather have the purpose of being useful to any blocker which supports it. It could already be used by AdGuard, requiring only the few lines of code to be added to AdGuard.

If this is of interest to you, we could re-work the project:

  • Find a icon which is not associated with a specific blocker;
  • Emphasize it can be used with any blocker which supports it (with an alphabetically-ordered list of blockers);

Advantages I see of one extension serving all blockers:

  • Can be improved with technical insights from more parties;
  • It become the official go-to extension to expose WebSocket connections to blockers;

Obviously, the root issue being fixed in Chromium would be the best, but so far the issue has been left untouched for years.

Mizzick added a commit that referenced this issue May 4, 2016
Mizzick added a commit that referenced this issue May 4, 2016
Mizzick added a commit that referenced this issue May 4, 2016
@ameshkov
Copy link
Member Author

ameshkov commented May 4, 2016

Hi @gorhill!

Thanks for notifying me! Just a few questions on this.

  1. Why do you create an image instead of using a window.postMessage?

window.postMessage just looks like a more "generic" way of doing this. Also, as I understand, we have the same issue with FF add-on, nsIContentPolicy events do not fire on WS requests. Not sure about nsIObserver yet.

Btw, I just got an idea. Both ways are currently vulnerable as site owner can easily override Image constructor or window.postMessage and make us think that request should not be blocked. So I guess we should save original function inside a local variable.

  1. Don't you think that in the end we will be forced to incorporate this feature in extension? I just see that more and more ad networks use WS. Instead of having a separate extension we could work on some kind of a shared library together. What do you think about it?

Obviously, the root issue being fixed in Chromium would be the best, but so far the issue has been left untouched for years.

Yeah, that's highly unlikely:)

Mizzick added a commit that referenced this issue May 5, 2016
@gorhill
Copy link

gorhill commented May 5, 2016

Why do you create an image instead of using a window.postMessage?

In the current case, using window.postMessage would mean script code in the web page itself communicating with script code in the main extension process. I don't know how to obtain the window instance of the main process of the extension from the web page itself.

we should save original function inside a local variable

Good point.

Don't you think that in the end we will be forced to incorporate this feature in extension?

I got skittish with this after I broke prominent sites last Monday with this solution. Granted, this was entirely my fault with a poorly tested and hald-assed implementation. But ideally yes, it's best if it's part of the extension itself, as I believe most users would not know they have to install another extension to gain the ability to block websocket connections.

Your idea of library is probably best in this case, it can be used as see fit by blocker extensions. For the time being, given how I broke things last Monday, I would stick to a companion extension on my side, but will end up relying on that shared library.

Currently I find it annoying to write the code in a string, but I can see no other way to ensure the WebSocket wrapper is injected at earliest in the web page. Probably best to have an extension building step to fill in that string from the lib.

Ideally, using web_accessible_resources is the correct and clean way to do this, but this would allow a web page to directly identify that a user has such or such blocker installed, thus adding bits to a fingerprint (on the other hand, I am sure you will agree with me that's it's easy for web pages to create a small script to make a pretty good guess about which blocker is in use -- each blocker has its specific effects on a page.)

So anyways I just spinned off the WebSocket wrapper code in a standalone file, which will be the library: https://github.com/gorhill/chromium-websocket-wrapper. I am pretty open if you rather have the project maintained elsewhere, or regarding js style, etc.

@gorhill
Copy link

gorhill commented May 5, 2016

we have the same issue with FF add-on, nsIContentPolicy events do not fire on WS requests. Not sure about nsIObserver yet

Yes, a HTTP observer can see WebSocket requests on Firefox.

@ameshkov
Copy link
Member Author

ameshkov commented May 5, 2016

Currently I find it annoying to write the code in a string, but I can see no other way to ensure the WebSocket wrapper is injected at earliest in the web page. Probably best to have an extension building step to fill in that string from the lib.

Look at what @Mizzick has done in the last commit:
https://github.com/AdguardTeam/AdguardBrowserExtension/pull/241/files

So, basically:

  1. websocket.js is included as a content script
  2. Everything about wrapping WebSocket is inside the "overrideWebSocket" function
  3. Then inside a content script he simply calls overrideWebSocket.toString:
    scriptsToApply.unshift('(' + overrideWebSocket.toString() + ')();');

@ameshkov
Copy link
Member Author

ameshkov commented May 5, 2016

I got skittish with this after I broke prominent sites last Monday with this solution.

Any idea which websites are good for WS wrapper testing?

@ameshkov
Copy link
Member Author

ameshkov commented May 5, 2016

So anyways I just spinned off the WebSocket wrapper code in a standalone file, which will be the library: https://github.com/gorhill/chromium-websocket-wrapper. I am pretty open if you rather have the project maintained elsewhere, or regarding js style, etc.

Let's just make it a bit more "modular" and contribute together (if you're ok with a hell of a lot of comments, i just like to comment every function).

Something like this:

(function(channel) {
    'use strict';

    var Wrapped = window.WebSocket;
    var toWrapped = new WeakMap();

    ....
    ....

    channel.onResponseReceived = onResponseReceived;
    channel.test();
})(imageChannel);
var imageChannel = (function() {
    var test = function() {
        var img = new Image();
        img.src =
              window.location.origin
            + '?url=' + encodeURIComponent(url)
            + '&ubofix=f41665f3028c7fd10eecf573336216d3';
        img.onload = this.onResponseReceived.bind(img, this, true);
        img.onerror = this.onResponseReceived.bind(img, this, false);
    }

    return { test: test };
})();

@gorhill
Copy link

gorhill commented May 5, 2016

I know of these two, they use websockets to retrieve images:

  • http://www.opensubtitles.org/en/search
  • http://thewatchseries.to/cale.html?r=aHR0cDovL2dvcmlsbGF2aWQuaW4veDBlM252eWk2MXh2

Be sure to test the sites which were reported broken (but fixed since then with the new wrapper code):

@gorhill
Copy link

gorhill commented May 5, 2016

if you're ok with a hell of a lot of comments, i am just used to comment every function

I completely agree, the only reason I removed the comments is because I had to stringify the code.

Also I see I left ubofix=, this should be changed to something more generic I suppose. The f41665f3028c7fd10eecf573336216d3 value is just a shasum of the original URL of the websocket issue on github. It can be anything really, as long as it's likely to never be used as a real URL elsewhere on a site.

@ameshkov
Copy link
Member Author

ameshkov commented May 5, 2016

@gorhill the "channel" abstraction purpose it to allow others to use any messaging, not just through an image. If @Mizzick is able to make window.postMessage work well, we'd better stick with it.

One more thing on the "image" way. User may turn images off for the websites, blocking WS as well.

So, in the library we can simply have two channel implementations, imageChannel and messageChannel

Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 5, 2016
Mizzick added a commit that referenced this issue May 6, 2016
@ameshkov ameshkov closed this as completed May 6, 2016
ameshkov pushed a commit that referenced this issue Dec 28, 2018
…1266-1_fix to feature/redesign

* commit 'c6ee14bfee51fa4045af5d4eed3a0ab0e587fd7d':
  feature/1266-1_fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants