Uassets: Scriptlets injection breaks some extensions

[jdoe1024]

Prerequisites

• [x] I verified that this is not a filter issue
  
  
  
  • Filter issues MUST be reported at filter issue tracker
    
    
  
  
  

[x] This is not a support issue or a question

• Support issues and questions are handled at /r/uBlockOrigin
  
  

[x] I performed a cursory search of the issue tracker to avoid opening a duplicate issue

• Your issue may already be reported.

I tried to reproduce the issue when...

• [ ] uBlock Origin is the only extension


• [x] uBlock Origin with default lists/settings


• [x] using a new, unmodified browser profile

[x] I am running the latest version of uBlock Origin

[x] I checked the documentation to understand that the issue I report is not a normal behavior

Description

Scriptlet injection is a nice feature to block ads JS code, but it impacts all scripts on the page. It is very common for browser extensions to inject scripts in web pages, but because of the scriptlet injection feature, some scripts injected by browser extensions are broken depending on the uBlock filters for the current web page. I'm mostly interested in TamperMonkey extension (see related tickets here and here) but it should impact other extensions as well by design.

A specific URL where the issue occurs

https://www.shameless.com/dmca.php (NSFW)

Steps to Reproduce

1. Install uBlock and TamperMonkey extensions on Chrome
2. Make sure everything is up to date (filters, extensions, etc.)
3. Go to given URL
4. Open developer console

Expected behavior:

No error.

Actual behavior:

An error message should appear in the console indicating TM cannot injects its sandbox (if not, reload the web page as it only happens when TM injects its script after uBlock applies its filters, which seems randomish). The message itself is something like Tampermonkey sandbox preparation failed. This usually is caused by a third-party extension. ReferenceError: xxxxxxx and is caused by the following filter in uBlock filter:

shameless.com##+js(aopr, document.dispatchEvent)

TM sandbox script references dispatchEvent (and a lot of other DOM methods/properties) so it is blocked by this filter:

Please note that this is not a specific filter issue, this is a design issue of the scriptlet injection feature. It should not interfere with scripts injected by extensions as they are safe by definition. I'm not sure how the scriptlets can detect that, maybe walking the stack trace to detect where the calling script comes from, or looking for a specific context variable (set by the other extensions for instance). If this is not possible, then there should at least exist a policy for scriptlet injection filters stating that builtin DOM methods/properties should not be targeted.

Your environment

• uBlock Origin version: 1.24.2 (latest)
• TamperMonkey version: 4.9 (latest)
• Browser Name and version: Chrome v79.0.3945.117
• Operating System and version: Windows 10

[mapx-]

@gorhill

[gorhill]

this is not a specific filter issue

Yes, this is a filter issue -- you are asking uBO to do the impossible or otherwise severely nerf itself.

Both the TamperMonkey code and the uBO filter code are executed in the page context, there is no way for uBO to ascert that the TamperMonkey code is from TamperMonkey and not from the page itself.

Such incompatibility have to be dealt on a per-site basis, and ultimately a user can choose to file an issue and hope another filter can be crafted, except the filter, or accept that TamperMonkey won't work on the page.

[lain566]

I replaced the old filter.

[uBlock-user]

Compatibility with other extensions is not taken into consideration(and shouldn't be expecting that filter lists maintainers do) when filters of this sort are created, only that it does what it is supposed to. This issue will occur with ABP/Adguard and every extensions that injects javascripts which trap calls to a js function, so not a uBO specific issue. TamperMonkey users should simply disable the filter that causes breakage.

[jdoe1024]

@gorhill What about something like that:

1. uBO generates a long random variable name when the browser starts.
2. It exposes that name to other extensions using the chrome.runtime.onMessageExternal feature (or equivalent for other browsers).
3. In uBO scriptlets, if a global variable with that name exists, then the scriptlet does not raise an error.
4. It is up to the other extensions to define that variable at the start of their injected scripts and delete it before the scripts exit (wrapping it in a try {} finally {} block for instance). That variable could even be used as a counter to allow re-entrant executions. Should the injected script forget to delete the variable, it will be the other extension fault, not uBO.

@uBlock-user I do understand that filters authors cannot make sure their filters are compatible with other extensions, which is why the only sustainable way for me to fix that is to modify uBO behavior. But if this is not possible, then the best workaround would be for these authors to not target DOM builtin properties/methods/objects in scriptlet injection filters when possible, as @lain566 did successfully 👍

[uBlock-user]

then the best workaround would be for these authors to not target DOM builtin properties/methods/objects in scriptlet injection filters when possible,

Compatibility with other extensions is not taken into consideration(and shouldn't be expecting that filter lists maintainers do) when filters of this sort are created, only that it does what it is supposed to.

Changing them all(document.dispatchEvent alone is used 58 times, not counting several others) to make TM working again is not something that will happen given the number is huge and shouldn't be expected either. Changing the filter here just fixes this one case. Your only option is to prioritise which extension you want to work and act accordingly.

[gorhill]

It exposes that name to other extensions using the chrome.runtime.onMessageExternal

uBO does not trust other extensions, so exchanging information with other extensions is not an option, and so is not pegging uBO's proper functioning to the dictate of any other extension.

uBO is no more at fault from trapping dispatchEvent in the page context than TamperMonkey is -- it's just an unfortunate interference. At least on uBO's side this occurs conditionally when a filter exists for a given site, while in TamperMonkey it's done unconditionally.

Both extensions just happen to do it because it's a requirement for their proper functioning. At most, we can ask filter list maintainers to try to avoid overriding built-in properties when they can, but in the end it's their call to do so -- this might no always be possible.

[okiehsch]

I am all in favour of not aborting dispatchEvent if another filter is able to fix the issue.

About the specific issue at shameless.com, i.e. the ads by https://www.exoclick.com
We tried different filters for example ExoLoader, ExoLoader.serve etc. but they changed their script all the time that's why we then added ##+js(aopr, document.dispatchEvent) for over 50 adult sites and we did not have to change the filters since then.
It does not break site functionality, so I am not in favour of changing them all, especially because I am pretty sure that if we would add filters like exoNoExternalUI38djdkjDDJsio96 for all sites they will again regularly update their script.