xiaoxiaoflood / firefox-scripts

userChromeJS / autoconfig.js and extensions
Mozilla Public License 2.0
1.03k stars 87 forks source link

Compatability with Future Versions of Firefox #64

Closed tomrittervg closed 2 months ago

tomrittervg commented 3 years ago

Hi; I'm a Firefox developer on the security engineering team. We're working to lock down unexpected behavior in the browser, and have seen evidence that these scripts are causing unexpected JS loads in the parent process. That's to be expected, that's exactly what they're going to to do - but the loads are happening despite us turning off our checking behavior if we think these scripts might be present.

I'm hoping you can help us figure out how this may be happening so we can avoid breaking your scripts when we start to enforce the checking behavior in the future (which would stop the script from loading, and silently break stuff.)

Specifically we check the general.config.filename pref on the main thread. If you're using some sort of userChromeJS or boostrapLoader that doesn't set that pref or you're loading extensions that ultimately use workers and load a JavaScript file off-main-thread we would start blocking things.

In the last ~1 month I've seen someone (literally one Windows user) triggering this using BackTrack Tab History on Nightly, but with users disabling Telemetry and the non-Windows reporting less verbose, it's possible this is happening more often.

So I was wondering:

1) Can you enable legacy stuff without setting general.config.filename? If yes, would you consider making setting this pref a requirement for these scripts to operate? 2) Do you know if any of the extensions (especially BackTrack Tab History) load scripts off-the-main thread (e.g. in workers)?

117649 commented 3 years ago

Do you mean something like let worker = new window.Worker('{script url}')?

tomrittervg commented 3 years ago

Do you mean something like let worker = new window.Worker('{script url}')?

Yes, that would mean you're using a worker in the extension and is probably indicative of tripping our alerts. If that's a common use in these extensions we (Mozilla) could look at general.config.filename on all threads instead of just main thread....

117649 commented 3 years ago

https://github.com/117649/Tab-Groups/blob/9c651d7cf72e9139b883f3bfbb19c3ba5eb416be/resource/modules/FavIcons.jsm#L202-L259 One place I know.

Would load frame script also tripping your alerts? That's a way more common use.

AlexVallat commented 3 years ago

Firstly, @tomrittervg , I'd just like to say thank you for reaching out with this. Even if Firefox does end up breaking our addons again, the fact that you've taken the time to look into the issue means a lot.

I'm the author of BackTrack Tab History, and can confirm that it does not use web workers, but it does use (delay load) frame scripts, loaded via globalMessageManager.loadFrameScript. It also uses messaging through globalMessageManager.addMessageListener and sendAsyncMessage both from and to the frame script.

xiaoxiaoflood commented 3 years ago

Hi,

Thank you for gently coming here to discuss a way to not break our scripts/extensions even though they aren't officially supported by Mozilla. I really appreciate it. :blush:

  1. Can you enable legacy stuff without setting general.config.filename?

Only through bootstrapLoader.xpi, a WebExtension Experiment to load bootstrapped legacy addons. But I don't really like it and I don't use it. userChromeJS (through general.config.filename) is better as it runs earlier so it has more powers. Also, userChromeJS can be installed on any Fx version (if you set general.config.sandbox_enabled = false), while Experiments are restricted to Nightly/DevEd/unbranded builds.

I didn't write a WebExt Experiment to load userChromeJS scripts and I fear some of them may not work properly that way, like Master Password+ that needs to lock all Fx windows (not just browser.xhtml) on startup before any content is displayed.

If yes, would you consider making setting this pref a requirement for these scripts to operate?

general.config.filename is already required to use userChromeJS (aka autoconfig, the JS equivalent to userChrome.css, that's why we call it userChromeJS and use chrome folder).

userChromeJS not only enables UserScripts (.uc.js files), I also added the bootstrapLoader code in it. That's what I use on my end, I don't use bootstrapLoader.xpi.

I don't use nor recommend using WebExt Experiment to enable legacy extensions/scripts, so as long as we can continue having userChromeJS with full privileges I'm fine. So no problem for me if you block Experiments when general.config.filename is not set.

xiaoxiaoflood commented 3 years ago
  1. Do you know if any of the extensions (especially BackTrack Tab History) load scripts off-the-main thread (e.g. in workers)?

Most of them use messageManager, like DownThemAll!. Some userChromeJS scripts too.

tomrittervg commented 3 years ago

Presently my dataset is restricted to Nightly, so it may be coming primarily from bootstrapLoader.xpi. I'm going to propose checking for xpinstall.signatures.required = false and if so, disable our checks. Hopefully that quiets down the activity. It's preferable to get the data down to near-zero, that means that we understood what the problem was and there's less chance of breaking something unknown!

The other possibility is workers; but given the example provided actually uses the chrome:// scheme (and we allow those) I'm feeling less sure of this. Still; I don't think it would be hard to introduce a static variable to check general.config.filename off-main-thread. (TBH I'm not sure why I didn't do that in the beginning.)

Finally, I'm going to try to increase our Telemetry to Beta and eventually Release. All of this will be in 1688800. Enabling Telemetry - if you disabled it - will help us to avoid breaking your stuff. :) The data I'm specifically collecting would be shown in the 'Events' section of about:telemetry with the key security.javascriptLoad (and security.evalUsage but that one has graduated to enforcement mode so if it was going to break something, it already did.) Feel free to ping me in the future to follow up on this - and if things did break for you, mozregression is a good tool to figure out what it was!

xiaoxiaoflood commented 3 years ago

Thanks. I've been using mozregression extensively since Fx 57 to track and fix breakages. Like your 1570681 did in Fx 71 to bootstrapLoader.xpi users. :stuck_out_tongue_winking_eye:

xiaoxiaoflood commented 3 years ago

So Fx 88 broke DownThemAll!: https://bugzilla.mozilla.org/show_bug.cgi?id=1694462

https://github.com/downthemall/downthemall-legacy/blob/84160d789062691de0ffd82bdc4ebc123b540203/modules/manager/chunk.js#L17-L38

I could simply rename Services.tm.newThread(0) by Services.tm.newNamedThread(''), but... JS doesn't have access to it. 😕

Would it be possible to remove [noscript] from newNamedThread, please?

Edit: I'll pass null to AsyncStreamCopier2. I hope that's OK. It works, at least.

117649 commented 3 years ago

Have anybody notice that script load use loadFrameScript is not executing on non-chrome tabs?

let mm = window.getGroupMessageManager("browsers");
    mm.loadFrameScript("chrome://browser/content/tab-content.js", true, true);

I tried same code as above but different script, can see script with getDelayedFrameScripts() but not in dev tool or trigger break point on them.

Did mozilla made some new restrictions?

xiaoxiaoflood commented 3 years ago

Working fine here both on remote and on non-remote tabs.

getGroupMessageManager('browsers').loadFrameScript('data:application/javascript,' + 
  encodeURIComponent('(' + (() =>
    console.log('hello from *' + content.document.title + '*')
  ).toString() + ')();'), true);

hello from *Advanced Preferences hello from *Add-ons Manager hello from *Add-ons for Firefox (en-US) hello from *Example Domain hello from *Google hello from *New Tab

117649 commented 3 years ago

I've added 'console.log('hello from ' + content.document.title + '')' as log at the first line of a real script in dev tool. Here is the comparison: image This is not good.

xiaoxiaoflood commented 3 years ago

tst.js:

console.log('hello from *' + content.document.title + '*')

Both

getGroupMessageManager('browsers').loadFrameScript('chrome://userchromejs/content/tst.js', true);

and

getGroupMessageManager('browsers').loadFrameScript('resource://userchromejs/utils/tst.js', true);

work for me on every tab.

117649 commented 3 years ago

This is just weird.

Do you have any preference setting in About:config that is special?

xiaoxiaoflood commented 3 years ago

No. It's something on your side. You can open a fresh new profile in latest Nightly with no user.js, no Policies... then install userChromeJS and finally do what I did in my previous comment. It works as intended.

117649 commented 3 years ago

Ok, I think I know what is it. I'm using load extension form folder. It seems that hit some security barrier. Installed it again with a proper .xpi now it is fine.

onemen commented 2 years ago

@117649,

If you are using Mac then you can only load extensions from folder in Extensions folder

; Per-user and system-wide Extensions dir
  (allow file-read*
      (home-regex "/Library/Application Support/[^/]+/Extensions/")
      (regex "^/Library/Application Support/[^/]+/Extensions/"))

https://github.com/mozilla/gecko-dev/blob/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/security/sandbox/mac/SandboxPolicyContent.h#L283-L286

You can add a file with your extension ID in the profile extensions folder that point to another folder.

To test TabMixPlus, I added this line to a file with then name {dc572301-7619-498c-a57d-39143191b318} ~/Library/Application Support/Mozilla/Extensions/TabMixPlus/addon

I drop this file to any profile that I'm testing, and all the files for developing TabMixPlus are in the repository in ~/Library/Application Support/Mozilla/Extensions/TabMixPlus

117649 commented 2 years ago

@117649,

If you are using Mac then you can only load extensions from folder in Extensions folder

; Per-user and system-wide Extensions dir
  (allow file-read*
      (home-regex "/Library/Application Support/[^/]+/Extensions/")
      (regex "^/Library/Application Support/[^/]+/Extensions/"))

https://github.com/mozilla/gecko-dev/blob/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/security/sandbox/mac/SandboxPolicyContent.h#L283-L286

You can add a file with your extension ID in the profile extensions folder that point to another folder.

To test TabMixPlus, I added this line to a file with then name {dc572301-7619-498c-a57d-39143191b318} ~/Library/Application Support/Mozilla/Extensions/TabMixPlus/addon

I drop this file to any profile that I'm testing, and all the files for developing TabMixPlus are in the repository in ~/Library/Application Support/Mozilla/Extensions/TabMixPlus

Never used a Mac.

tomchiverton commented 2 years ago

Ubuntu now uses Snaps, which can't / don't appear to read /usr/lib/firefox so nothing works any more :(

onemen commented 1 year ago

@xiaoxiaoflood

Mozilla fixed the issue (Bug 1785278, fb762c3) with Firefox snap for Linux, by adding an new path to add config files. Starting with Firefox 108 (currently in Nightly) you can add the files extracted from this zip file to these locations:

You may have to create the sub folders /defaults/pref