jpgohlke / twitch-chat-filter

58 stars 37 forks source link

Added support for Greasemonkey 2.0. #160

Closed MattiasBuelens closed 10 years ago

MattiasBuelens commented 10 years ago

Greasemonkey 2.0 made some backwards incompatible changes regarding the unsafeWindow. In short, directly writing non-primitive values to unsafeWindow from the script's context into the page's context is no longer supported. This way, other scripts on the page are prevented from breaking into the sandbox by calling a function or modifying an object added by a user script.

In order to safely write values to the page, user scripts now need to use cloneInto, exportFunction and/or createObjectIn to export safe object clones and/or export functions which accept and return safe values.

The first part of the solution is to use our own instance of jQuery for DOM manipulation. The DOM is still shared between the script's context and the page's context, and by running in noConflict mode we prevent our instance from interfering with the instance already loaded on the page. Because our instance runs in our sandbox, we don't need to worry about exporting objects or cloning functions to run in the page's context.

The second part of the solution is a bit more tricky, since we need to inter-operate with Twitch's JS code on the page: for example we need to modify App.Room.prototype for addMessage/send and we need to load emoticons with Twitch.api.get for the emoticons filter. In these cases, we need to export some of our functions in the sandbox to the page so they can be called from the page's context. These functions are exported to window.TppChatFilterBridge on the page. Additionally, we need to inject some glue code into page to wire the bridge up with Twitch's functions. However, the rest of our code still runs inside the sandbox.

This is an alternative solution for #157. @hugomg implemented a different solution in #158, in which all of the script's code is injected into the page. This is an easier and simpler solution, but it's less secure.

MattiasBuelens commented 10 years ago

A word of warning: this still needs more testing. :stuck_out_tongue:

We also might want to optimize for the non-Greasemonkey use case, for example simply run a function instead of injecting it in a script.

MattiasBuelens commented 10 years ago

Correct, user scripts run in a privileged context whereas the page scripts run in a less privileged context. Thus, the security concern is about a page script (which might be injected through an XSS attack) abusing a user script to do things in the privileged context. The updated sandbox forces you to use secured mechanisms when you try to do things across the two contexts.

I agree, we should run everything in the less privileged context. We don't need anything from the privileged context: we don't need cross-domain AJAX requests with GM_xmlhttpRequest, or set the clipboard contents with GM_setClipboard. So this pull request should definitely be closed.

As for the debugging issue, apparently there is a sourceURL annotation (similar to the sourceMappingURL which you can add to a dynamically loaded script's contents to indicate the source of the script. The debuggers of some browsers (well, just Chrome right now I believe) support this annotation so the script shows up in the Scripts panel with line numbers and everything. I'd have to try it out to see if and how this works (we'd have to get an URL for the user script... not sure how).