1958232 - Allow eval in browser.xhtml for users with userChromeJs with a special pref

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P2)

Description

[Tom Schuster (MoCo)]

We allow eval even for system principals in nsContentSecurityUtils::IsEvalAllowed when the user is probably using some kind of userChromeJS modification (DetectJsHacks). We should also continue to allow eval in the context of browser.xhtml under the same conditions and not block it via its CSP.

Comment 1

[Tom Schuster (MoCo)]

Attachment: Bug 1958232 - Allow eval in browser.xhtml and other priviliged contexts with a special pref. r?tjr

Comment 2

[Simon Friedberger (:simonf)]

I mentioned this in other discussions about DetectJSHacks: Should we add an explicit pref for this? It's just a collection of other prefs anyway and the creators of these extensions seem to be at least somewhat responsive so adding an explicit pref to disable CSP checking and an explicit pref to allow eval - both with warnings that this is bad for Firefox security - sound like a good idea to me.

Comment 3

[Tom Schuster (MoCo)]

I think a pref has definitive upsides, like being explicit and give us the opportunity to use a descriptive name like unsafe_allow_evil_eval_in_privileged_code. I my mind there is also the downside of us seemingly legitimizing this.. I don't think there is much point in adding a pref just for this case as long as we still allow other dangerous eval uses due to DetectJSHacks. Actually requiring a pref for all unsafe eval uses is something I strongly prefer. I just don't really want to risk the potential fallout right now.

Comment 4

[Frederik Braun [:freddy]]

Reminds me how we have had the security.turn_off_all_security_so_that_viruses_can_take_over_this_computer preference that was consequently enabled by a relatively reduced write-what-where primitive in an exploit such that all security was gone in a poof (cf. bug 984012).

If anything, I would like to make it a comparably hard tell-tale sign that people want to disable our security protections.

Having a pref like general.config.filename or autoadmin.global_config_url set to a valid value that was actually tested at start up (do we test at start up?) sounds like a better hurdle.

Similarly, we have seen attackers overwrite functions that perform security checks with a "return true", so there's a tactical piece of that which may optimize for specific exploits rather the actual vulnerabilities. Amy Burnett gave a great talk at BlueHat 2020 where she made the case that attackers may not need a sandbox escape, if e.g., the Same-Origin Policy can just be disabled or the Principal can be written to SystemPrincipal etc.

Comment 5

[Shane Hughes [:aminomancer]]

I wouldn't mind another pref, but it seems superfluous when we're already executing arbitrary code through autoconfig. I think most users are using script loaders that hop from the install folder to the profile folder, which seems like basically the same security issue since the profile folder doesn't have the same write protections.

Btw, can't you still basically eval in browser.xhtml? Services.scriptLoader.loadSubScript("data:text/javascript," + unsafeVariableString) is working for me. The same pattern with ChromeUtils.compileScript(...).then(s => s.executeInGlobal(this)) is also working. Aren't those practically the same, in security terms?

Comment 6

[Tom Schuster (MoCo)]

Btw, can't you still basically eval in browser.xhtml? Services.scriptLoader.loadSubScript("data:text/javascript," + unsafeVariableString) is working for me. The same pattern with ChromeUtils.compileScript(...).then(s => s.executeInGlobal(this)) is also working. Aren't those practically the same, in security terms?

Yes. We can't fix everything at once of course. Additionally there are natural limits of what we can do, because we can't break functionality that is required for the browser to function. But those two seem like obvious candidates to investigate, see bug 1938905, thanks!

Comment 7

[Shane Hughes [:aminomancer]]

(In reply to Tom Schuster (MoCo) from comment #6)

Yes. We can't fix everything at once of course. Additionally there are natural limits of what we can do, because we can't break functionality that is required for the browser to function. But those two seem like obvious candidates to investigate, see bug 1938905, thanks!

Makes sense. Will that also go behind the same nsContentSecurityUtils::HasJSHacks check? Right now, the userchromeJS community is mostly using script loaders with ChromeUtils.importESModule. So if it goes down for a long time before a workaround is made available, I suspect many userchromeJS users will roll back to an old version or a different browser, and then it may be hard to get them back on the latest Firefox.

Comment 8

[Tom Schuster (MoCo)]

Right now, the userchromeJS community is mostly using script loaders with ChromeUtils.importESModule. So if it goes down for a long time before a workaround is made available, I suspect many userchromeJS users will roll back to an old version or a different browser, and then it may be hard to get them back on the latest Firefox.

Do you mean code using eval inside a module loaded via ChromeUtils.importESModule or are you talking about using importESModule("data:text/javascript,alert('hello world')" for dynamic script execution?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6732c849701
Allow eval in browser.xhtml and other priviliged contexts with a special pref.  r=tjr

Comment 10

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b6732c849701