1329731 - Disabling JavaScript breaks Web Extensions

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[Alexei]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.100 Safari/537.36

Steps to reproduce:

Privacy Badger, a privacy addon newly converted to Web Extensions, breaks for users who disable JavaScript in Firefox.

Please see https://github.com/EFForg/privacybadger/issues/1098#issuecomment-270967040 for the original Privacy Badger issue.


Actual results:

According to lcrouch, background scripts of Web Extensions seem to stop running.


Expected results:

Disabling JavaScript in Firefox should not break Web Extensions addons.

Comment 1

[Andy McKay]

We generally consider an extension part of the browser and turning off Javascript should affect content pages, but not parts of the browser. So we'll make the assumption that the extension is part of the browser and background pages should not be affected.

Should it affect content scripts?

The other question, is this possible to do?

I'm not sure who to ask on this, to be honest, so I'll start with dveditz... :)

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

If I'm not wrong, at least looking at `PrincipalImmuneToScriptPolicy` defined in XPCJSContext.cpp (http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCJSContext.cpp#333-362), it seems to me that the system principal and the expanded principal (which are used by the content scripts) gets a "free pass" and they should not be affected by the script policy settings.

So, my guess is that currently the content scrips are not currently affected (their javascript still run), but all the webextension pages, e.g. the extensions tab and background pages, the popup window etc., are affected (their javascript don't run when globally disabled).

how about adding "principals with a moz-extension schema" to the list of principals immune to the script policy in the method linked above?

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1329731 - Add moz-extension to the principals immune to script policy.

Review commit: https://reviewboard.mozilla.org/r/105112/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105112/

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

I've just attached a patch with the change proposed in Comment 2, I did some tests locally and it looks like it is enough to make the WebExtensions pages immune to "javascript.enabled" set to false from about:config.

Pushed also to try to check if it affects any of the existent tests:

- https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4db22c9cc703e9a6cd0aa0f8e47d1a678123eaf

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Luca Greco [:rpl] from comment #2)
> the expanded principal (which are used by the content scripts) gets a "free pass"

I just noticed that there was a typo in comment 2:
I meant to write "which is used", and it means that "content scripts use an expanded principipal".

Comment 6

[Alexei]

> my guess is that currently the content scrips are not currently affected
Yep, this agrees with what lcrouch reported in the GitHub issue: https://github.com/EFForg/privacybadger/issues/1098#issuecomment-271109418

Comment 7

[Daniel Veditz [:dveditz]]

(In reply to Andy McKay [:andym] from comment #1)
> We generally consider an extension part of the browser and turning off Javascript should affect
> content pages, but not parts of the browser. So we'll make the assumption that the extension
> is part of the browser and background pages should not be affected.

That's what we've assumed for old-style/XUL add-ons.

> Should it affect content scripts?

It's still part of the extension. That we call them "content scripts" is an implementation detail.

Any scripts injected into the page itself should fail (adding event handlers/on* attributes to elements?), but extended-browser scripts running in a sandbox separate from the web content is fine and expected. The web extension might be adding security vulnerabilities if it's poorly written, but turning off web content javascript is not the fix for that.

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8827425 [details]
Bug 1329731 - Add moz-extension to the principals immune to script policy.

https://reviewboard.mozilla.org/r/105112/#review107566

::: js/xpconnect/src/XPCJSContext.cpp:352
(Diff revision 1)
>      aPrincipal->GetURI(getter_AddRefs(principalURI));
>      MOZ_ASSERT(principalURI);
> +
> +    // WebExtension principals gets a free pass.
> +    bool isWebExtension;
> +    nsresult rv = principalURI->SchemeIs("moz-extension", &isWebExtension);

Please check `principal->GetAddonId` instead.

Comment 9

[Bobby Holley (:bholley)]

Not sure what info is needed here. The patch looks sensible.

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8827425 [details]
Bug 1329731 - Add moz-extension to the principals immune to script policy.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105112/diff/1-2/

Comment 11

[Luca Greco [:rpl] [:luca] [:lgreco]]

Comment on attachment 8827425 [details]
Bug 1329731 - Add moz-extension to the principals immune to script policy.

https://reviewboard.mozilla.org/r/105112/#review107566

> Please check `principal->GetAddonId` instead.

Sure, patch updated as suggested.

Comment 12

[Bobby Holley (:bholley)]

Comment on attachment 8827425 [details]
Bug 1329731 - Add moz-extension to the principals immune to script policy.

https://reviewboard.mozilla.org/r/105112/#review113438

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cf2c9494288
Add moz-extension to the principals immune to script policy. r=bholley

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5cf2c9494288

Comment 15

[serfawd]

How to fix this in the version ESR 52.2.1?

Comment 16

[William Budington]

This is an important fix to backport to ESR.  Many extensions are just now making the migration to WebExtensions.  This issue is relatively obscure, and users expect their extensions to behave as expected if JavaScript is turned off. This makes the transition to WebExtensions harder.

I am the maintainer of HTTPS Everywhere, a popular Firefox extension. Because of this issue users on ESR with JavaScript turned off (security-conscious users, for example) have seen HTTPS Everywhere failing to function.  This includes Tor Browser users (which is built from the ESR source) who have strict security settings.

Please backport this to ESR.