javascript.enabled and docShell.allowJavascript do not disable all event handlers

RESOLVED FIXED in mozilla27

Status

()

Core
DOM: Events
--
major
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: Mike Perry, Assigned: bz)

Tracking

(Depends on: 1 bug, {privacy})

unspecified
mozilla27
privacy
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9 +

Firefox Tracking Flags

(firefox5-, firefox6-, blocking2.0 -)

Details

(Whiteboard: [firebug-p3], URL)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11) Gecko/20071222 Remi/2.0.0.11-1 Firefox/2.0.0.11
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.11) Gecko/20071222 Remi/2.0.0.11-1 Firefox/2.0.0.11

Javascript event handlers registered with addEventListener are not disabled when either javascript.enabled or docShell.allowJavascript are toggled. Other types of event handlers, such as those registered via the DOM event handler attributes are properly canceled.

Reproducible: Always

Steps to Reproduce:
1. open about:config
2. Visit http://fscked.org/transient/firefoxjsbug.html
3. Click on the page
4. Toggle javascript.enabled to false
5. Click on the firefox bug page again. Observe that some event handlers have been disabled, others have not
Actual Results:  
While some types of event listeners are blocked, other event listeners still fire.

Expected Results:  
No client-side javascript code should execute at all.

This bug is critical to Tor security. The Torbutton extension (https://torbutton.torproject.org/dev/) relies on the ability to prevent pages from executing Javascript and other dynamic content once Tor has been disabled, to prevent pages from performing network activity after the user has changed Tor state.
Confirming bug.

Have you considered adapting the "session restore" functionality and simply closing existing pages, then resurrecting the user's state when they turn off the Tor button? Even if you kill off the remaining event listeners for existing pages I'd still be nervous about letting Tor users interact with them.
Component: Security → DOM: Events
Product: Firefox → Core
QA Contact: firefox → events
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

10 years ago
What we could probably do is make AutoScriptEvaluate::StartEvaluating return an error if script can't be executed on that cx with the relevant principal.  That somewhat relies on the right cx being used, but we certainly do that with the cx pusher for event listeners.

Unfortunately, other callbacks into JS may not be so lucky (XMLHttpRequest readystate handlers come to mind, though there could also be issues in any DOM API that allows passing in page-implemented callbacks and might call them async... Or even that doesn't return for a long time (e.g. one of the callbacks does sync XMLHttpRequests or some such).  With this last, for example, treewalker's nodefilters could be attacked.  Or the namespace resolver from the proposed DOM API for Selectors.  Or any of a number of other callbacks.  I guess in the long-running situation the calling cx will still be on the stack, so things might be ok and we only need to worry about propagating the right cx into async callbacks...

That would only affect the docshell setting and per-site settings, though.  If we fail to get the right cx but are really async, we'll get the safe context and that's affected by the general "turn off JS" pref.  So at least as a first approximation doing the checks in StartEvaluating would work.

That said, nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject does an OBJ_GET_PROPERTY before calling StartEvaluating.  That doesn't seem safe...  brendan, am I missing something there?
(Assignee)

Comment 3

10 years ago
Requesting blocking, but I'm not completely sure it should, to be honest...
Flags: blocking1.9?
Not blocking as this is how we've always behaved and it's late in the release cycle. But wanted as it would be nice to have if someone came up with a safe fix.
Flags: wanted1.9+
Flags: blocking1.9?
Flags: blocking1.9-
(Reporter)

Comment 5

10 years ago
Would like to point out that many people do depend on this functionality outside of Tor users. Users of Javascript toggling extensions such as QuickJava come to mind as well.
Keywords: privacy
BZ or Jonas, do you have anyone in mind who could work on this?
Smaug might be a good candidate.
Assignee: nobody → Olli.Pettay
Or jst maybe. Jst: is there any way we can disable this on a javascript level? So that no matter which entry points we miss (what about timers) it'll still be blocked?

Comment 9

8 years ago
Any progress?

Comment 10

8 years ago
any news during the last few years?
(Assignee)

Comment 11

8 years ago
It's a pretty low priority.  I suspect that in the end someone who actually cares about dynamically enabling/disabling JS while a site is live will need to step up and fix this.  I'm happy to point to relevant code as needed.

Comment 12

8 years ago
docShell.allowJavascript = false;

doesn't prevent JavaScript in the onload="" attribute of images either.  This doesn't just affect websites, but XUL <editor>s as well.  (ScribeFire, in particular.)

I've filed a separate bug for this, but it might be better included in this bug.  I hope someone will fix it, because it greatly impacts major features of the <editor> element, including image drag and drop.

https://bugzilla.mozilla.org/show_bug.cgi?id=524329
(Assignee)

Comment 13

8 years ago
Yeah, that's different from this bug, since comment 0 explicitly says that attribute on* handlers _are_ prevented in the case this bug is filed about.

Updated

8 years ago
Whiteboard: [firebug-p1]

Comment 14

8 years ago
It seems that jsdIStackFrame.executionContext.scriptsEnabled may also be a victim.

Comment 15

8 years ago
We added this code to Firebug 1.5:
  context.eventSuppressor = context.window.getInterface(Ci.nsIDOMWindowUtils);
  if (context.eventSuppressor)
      context.eventSuppressor.suppressEventHandling(true);
in addition to 
  executionContext.scriptsEnabled = false;
when we stop on a breakpoint. 

This immediately solved one bug and we've not seen bad side effects. The only funny business was puzzing: if we call context.window.getInterface() after returning from the debuggers nested event loop, it fails. That is why we save the interface ref in context.eventSuppressor so we can use that to free the window after we return.
Whiteboard: [firebug-p1] → [firebug-p3]
(Reporter)

Comment 16

8 years ago
(In reply to comment #15)
> We added this code to Firebug 1.5:
>   context.eventSuppressor = context.window.getInterface(Ci.nsIDOMWindowUtils);
>   if (context.eventSuppressor)
>       context.eventSuppressor.suppressEventHandling(true);

John, I've tried doing this call on the contentWindow attribute of the relevant tabbrowser.docShell attribute, but it is disabling all hotkeys in the entire browser window. For example, control-t will not cause that browser window to open a new tab if the tab itself is the currently selected widget (as opposed to the URL bar, for example).

Did you notice this side effect as well? If not, which window are you actually calling suppressEventHandling on?
(In reply to comment #16)
> ...but it is disabling all hotkeys in the entire
> browser window.
...
> Did you notice this side effect as well?

(Not that this is related to this bug, but) preventing all hotkeys if the focused windows has events suppressed is by design, not a side effect.

Comment 18

8 years ago
Mike, mozilla.dev.platform is a great place to ask such questions. And I'll confirm what Olli said as what we also see.
(Reporter)

Comment 19

8 years ago
My apologies. I was just trying to keep some historical record for other people who hit this bug and try the workaround, since it is going on 3 years old, and I've long since lost any hope for an actual fix :)

I will try to investigate more and if needed continue workaround discussion somewhere else...

Updated

7 years ago
Flags: blocking1.9.0.19?
Flags: blocking1.9.0.19?

Comment 20

7 years ago
Status needs to be changed to Confirmed.

Updated

7 years ago
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
OS: Linux → All
Hardware: x86 → All
Not blocking 1.9.3 on this bug.
blocking2.0: ? → -
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---

Comment 22

6 years ago
The Tor Project / Electronic Frontier Foundation is paying to have this bug fixed.

"If you know C++ and/or Firefox internals, we should be able to pay you for your time to address these issues and shepherd the relevant patches through Mozilla's review process."

Source: https://blog.torproject.org/blog/web-developers-and-firefox-hackers-help-us-firefox-4

Comment 23

6 years ago
The original test case no longer works. But if it did we could see if Firebug 1.7 can block the events. I guess yes, we use nsIDOMWindowUtils suppressEventHandling(true).

Updated

6 years ago
tracking-firefox5: --- → ?
tracking-firefox6: --- → ?

Updated

6 years ago
Flags: blocking1.9-
tracking-firefox5: ? → -
tracking-firefox6: ? → -

Comment 24

5 years ago
Created attachment 651688 [details]
Minimal testcase

Since the original test case has disappeared, I took the liberty of making a new one.

Note that this isn't just a privacy issue, it's also a performance issue: sometimes you need to disable Javascript to prevent the browser from slowing to a crawl. The current bug means that it's never possible for the user to completely disable Javascript when it was previously enabled, unless he reloads ALL open pages.
Duplicate of this bug: 812056
Duplicate of this bug: 652049
(Assignee)

Updated

5 years ago
Depends on: 580070

Updated

4 years ago
Depends on: 971650
(Assignee)

Comment 27

4 years ago
Fixed by bug 862627.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Depends on: 862627
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
No longer depends on: 971650
Assignee: bugs → bzbarsky
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.