Last Comment Bug 409737 - javascript.enabled and docShell.allowJavascript do not disable all event handlers
: javascript.enabled and docShell.allowJavascript do not disable all event hand...
Status: RESOLVED FIXED
[firebug-p3]
: privacy
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: unspecified
: All All
: -- major with 12 votes (vote)
: mozilla27
Assigned To: Boris Zbarsky [:bz] (Out June 25-July 6)
:
Mentors:
http://fscked.org/transient/firefoxbu...
: 652049 812056 (view as bug list)
Depends on: ParisBindings 862627
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-24 15:18 PST by Mike Perry
Modified: 2014-02-12 13:57 PST (History)
30 users (show)
jonas: wanted1.9+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-
-


Attachments
Minimal testcase (956 bytes, text/plain)
2012-08-14 04:23 PDT, Champignon
no flags Details

Description Mike Perry 2007-12-24 15:18:58 PST
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.
Comment 1 Daniel Veditz [:dveditz] 2007-12-24 16:28:37 PST
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.
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-12-24 20:11:52 PST
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?
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2007-12-24 20:17:44 PST
Requesting blocking, but I'm not completely sure it should, to be honest...
Comment 4 Jonas Sicking (:sicking) PTO Until July 5th 2008-01-03 15:29:37 PST
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.
Comment 5 Mike Perry 2008-02-22 00:44:47 PST
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.
Comment 6 Brandon Sterne (:bsterne) 2008-02-22 16:37:58 PST
BZ or Jonas, do you have anyone in mind who could work on this?
Comment 7 Jonas Sicking (:sicking) PTO Until July 5th 2008-02-22 18:51:29 PST
Smaug might be a good candidate.
Comment 8 Jonas Sicking (:sicking) PTO Until July 5th 2008-02-22 19:28:39 PST
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 Tomas 2009-05-27 07:16:25 PDT
Any progress?
Comment 10 Levente Farkas 2009-09-20 14:42:37 PDT
any news during the last few years?
Comment 11 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-09-20 17:53:17 PDT
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 Christopher Finke 2009-10-24 19:23:48 PDT
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
Comment 13 Boris Zbarsky [:bz] (Out June 25-July 6) 2009-10-25 04:46:36 PDT
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.
Comment 14 John J. Barton 2009-11-05 08:02:14 PST
It seems that jsdIStackFrame.executionContext.scriptsEnabled may also be a victim.
Comment 15 John J. Barton 2009-11-13 09:16:22 PST
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.
Comment 16 Mike Perry 2010-03-18 03:57:17 PDT
(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?
Comment 17 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2010-03-18 04:43:15 PDT
(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 John J. Barton 2010-03-18 08:34:14 PDT
Mike, mozilla.dev.platform is a great place to ask such questions. And I'll confirm what Olli said as what we also see.
Comment 19 Mike Perry 2010-03-18 15:32:15 PDT
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...
Comment 20 obsolete.fax 2010-05-19 10:25:18 PDT
Status needs to be changed to Confirmed.
Comment 21 Johnny Stenback (:jst, jst@mozilla.com) 2010-05-26 15:55:50 PDT
Not blocking 1.9.3 on this bug.
Comment 22 shawn.sumin 2011-03-31 15:53:04 PDT
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 John J. Barton 2011-03-31 16:04:04 PDT
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).
Comment 24 Champignon 2012-08-14 04:23:38 PDT
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.
Comment 25 Matthias Versen [:Matti] 2012-11-14 23:21:40 PST
*** Bug 812056 has been marked as a duplicate of this bug. ***
Comment 26 Matthias Versen [:Matti] 2012-11-14 23:22:24 PST
*** Bug 652049 has been marked as a duplicate of this bug. ***
Comment 27 Boris Zbarsky [:bz] (Out June 25-July 6) 2014-02-12 08:05:51 PST
Fixed by bug 862627.

Note You need to log in before you can comment on or make changes to this bug.