Bug 192753 – User scripts are run even when JavaScript is disabled in settings, but WebPageProxy::runJavaScript is blocked

• Component: Security
• Status: NEW

[Carlos Garcia Campos]

I'm not sure whether this on purpose or not, but I've noticed that injected user scripts are run even when js is disabled in settings. Is the setting supposed to only disable scripts included in document? If that's the case, should we allow WebPageProxy::runJavaScript() when js is disabled in settings too? I was considering to add a new setting to allow js only for isolated worlds, so that applications like mail clients can disable js, but still use custom js for internal stuff using isolated worolds.

[Radar WebKit Bug Importer]

<rdar://problem/46773520>

[Darin Adler]

Just curious about the meaning of this in macOS Safari. What in the Safari app UI is an "injected user script"?

[Michael Catanzaro]

I don't know about Safari, but in Epiphany user scripts are used only to implement internal Epiphany features. So they're all trusted, and certainly not exposed in the UI. Hence I'm not personally concerned about them executing even when JS is disabled, and I wouldn't necessarily see this as a security issue unless we're aware of some actual application in which this behavior would be unexpected (unlikely?). I'm more concerned by the mismatch where runJavaScript() fails when JS is disabled but user scripts still run. That makes no sense from an API design perspective, and we should fix it either by allowing the runJavaScript() JS to run when JS is disabled, or by blocking user scripts when JS is disabled.

My vote would be to allow runJavaScript() when JS is disabled, because it reflects the client app making a choice to affirmatively run JS. The application has total control over the enable JS setting, after all, so it's hard to see how this would be a security issue. It could just not call runJavaScript() if it wants the call to be dependent on the setting. I'm pretty sure 100% of Epiphany's use of runJavaScript() are desired to work even if JS is disabled, and failure to execute the JS is almost never what apps are expecting; I actually had to remove the enable JavaScript setting from the Epiphany's UI specifically because there's no way to make runJavaScript() bypass that setting and too many internal Epiphany features are implemented using runJavaScript(). If we instead choose to break existing usage of user scripts, then that will probably just break apps without much benefit, and we still have to find a way to allow browsers to bypass the setting, e.g. by exposing a runJavaScriptNoReallyIMeanIt().

[Carlos Garcia Campos]

Epiphany doesn't use user scripts for its internals (except for the firefox sync thing). With user scripts I mean WebCore::UserScript, injected in wk2 with WebUserContentControllerProxy::addUserScript(). What epiphany uses for its internal stuff is JSC API to add its own js code on window object cleared. There are several APIs using js that we would need to make it work even when js is disabled in settings:

 - run_javascript
 - user scripts
 - user script message handler
 - code injected on window object cleared

And of course any internal use of js in WebKit like media controls. Those are already allowed because ScriptController::evaluate() is used, like for the user scripts, instead of ScriptController::executeScript() that is used for run_javascript and doing the settings check.

[Carlos Garcia Campos]

Ping. Does anybody know what the expected behavior should be? Should we disallow user scripts when js is disabled in settings? or should we allow run_javascript instead?

[Darin Adler]

Geoff, this seems like a question you might be good at answering.

[Geoffrey Garen]

My suggestion for Mail clients is to use the "JavaScriptMarkupEnabled" setting (rather than the "JavaScriptEnabled" setting).

JavaScriptMarkupEnabled allows all kinds of JavaScript execution in all worlds, but it forbids "<script>", "javascript:", "onclick=" and related kinds of markup from turning into executable scripts. JavaScriptMarkupEnabled should be sufficient to defend against scripts in arbitrary content because it takes away the content's entrypoint to any kind of script execution.

I also think JavaScriptMarkupEnabled is slightly more secure than just disabling JavaScript execution in some contexts while enabling it in others. JavaScriptMarkupEnabled solves the risk that the privileged context will copy and paste or otherwise process markup in a way that re-vivifies the attacker's JavaScript in classic "Reflected" XSS / injected script attack style.

As for WebCore::UserScript, the exact meaning of "run JavaScript" APIs in the context of a "don't run JavaScript" setting is admittedly complicated. I don't think we've historically treated it as a goal for the JavaScriptEnabled setting to prevent native APIs from running scripts, so I could imagine enabling WebCore::UserScript and WebPageProxy::runJavaScript even when JavaScriptEnabled is false. I guess this kind of confusion is another reason I prefer using JavaScriptMarkupEnabled over JavaScriptEnabled.

[Carlos Garcia Campos]

Thanks Geoffrey and Darin. I didn't even know we had JavaScriptMarkupEnabled setting. That's indeed better than adding yet another setting to allow js in isolated worlds. I'll expose that setting in GTK/WPE APIs. I still think that APIs to run js should be consistent to each other though, and I agree they all can be considered trusted, so it's probably a good idea to not block WebPageProxy::runJavaScript().

[Michael Catanzaro]

I'm going to make this public since it's a design issue rather than an exploitable security issue.

[Michael Catanzaro]

(In reply to Geoffrey Garen from comment #7)
> As for WebCore::UserScript, the exact meaning of "run JavaScript" APIs in
> the context of a "don't run JavaScript" setting is admittedly complicated. I
> don't think we've historically treated it as a goal for the
> JavaScriptEnabled setting to prevent native APIs from running scripts, so I
> could imagine enabling WebCore::UserScript and WebPageProxy::runJavaScript
> even when JavaScriptEnabled is false. I guess this kind of confusion is
> another reason I prefer using JavaScriptMarkupEnabled over JavaScriptEnabled.

So I don't think we need any changes to WebCore::UserScript.

My vote is to change WebPageProxy::runJavaScript to work even when JavaScriptEnabled is false.