Disabling JavaScript breaks Web Extensions

RESOLVED FIXED in Firefox 54

Status

P2
normal
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: alexeiatyahoodotcom+mzllbgzll, Assigned: rpl)

Tracking

(Blocks: 1 bug)

50 Branch
mozilla54

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: triaged)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.

Updated

2 years ago
Flags: needinfo?(amckay)

Comment 1

2 years ago
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... :)
Flags: needinfo?(amckay) → needinfo?(dveditz)
(Assignee)

Comment 2

2 years ago
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 hidden (mozreview-request)
(Assignee)

Comment 4

2 years ago
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
(Assignee)

Comment 5

2 years ago
(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".
(Reporter)

Comment 6

2 years ago
> 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
(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.
Flags: needinfo?(dveditz)

Updated

2 years ago
Assignee: nobody → lgreco
Priority: -- → P2
Whiteboard: triaged

Comment 8

2 years ago
mozreview-review
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.
Flags: needinfo?(bobbyholley)
Not sure what info is needed here. The patch looks sensible.
Flags: needinfo?(bobbyholley)
Comment hidden (mozreview-request)
(Assignee)

Comment 11

2 years ago
mozreview-review-reply
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.
(Assignee)

Updated

2 years ago
Attachment #8827425 - Flags: review?(bobbyholley)

Comment 12

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

https://reviewboard.mozilla.org/r/105112/#review113438
Attachment #8827425 - Flags: review?(bobbyholley) → review+
(Assignee)

Updated

2 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed

Comment 13

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5cf2c9494288
Add moz-extension to the principals immune to script policy. r=bholley
Keywords: checkin-needed

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5cf2c9494288
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54

Updated

2 years ago
Blocks: 1226547

Comment 15

2 years ago
How to fix this in the version ESR 52.2.1?
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.
Duplicate of this bug: 1405627

Updated

8 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.