Proposal to drop Tor Browser's plugin patches
Tor Browser has three patches related to blocking plugins:
- legacy/trac#3547 (closed) adds a function that whitelists the flash plugin only and excludes loading all other plugins
- legacy/trac#8312 (closed) hides the link to "Manage plugins" when the plugin is disabled
- legacy/trac#10280 (moved) adds a UI for enabling/disabling plugins in the add-ons page
These patches were introduced when Flash was still in fairly wide use. But since then, Flash has been disabled by default in Firefox, and is replaced on a substantial number of websites by HTML5 video and JavaScript. Furthermore, we want to strongly discourage users from using Flash as there is a significant risk that it will bypass the proxy or expose the user to tracking or security vulnerabilities.
First, from what I can see, when the pref plugin.disable
is set to true (as it is in browser/app/profile/000-tor-browser.js
), all plugins (including Flash) are blocked from ever loading into the Firefox process. Therefore the code in our legacy/trac#3547 (closed) is never exercised.
Second, legacy/trac#10280 (moved) only makes it more likely for the user to set "plugin.disable" to false, by exposing that pref in the UI.
Finally, legacy/trac#8312 (closed) seems unnecessary because, when "plugin.disable" is true, no "Manage plugins" link appears. Instead, the only message is "A plugin is needed to display this content." Also, various popular video sites, such as YouTube and Vimeo, now use HTML5 video without any complaints about missing Flash.
So I would suggest we can drop these three patches. Instead we might consider a couple of UI tweaks to improve user safety:
- Hide the Plugins section of about:addons altogether to prevent the user from even considering loading any plugins
- Change the plugin failure message to "A plugin would be needed to display this content. For security reasons, Tor Browser does not support plugins."
I think both of these changes could be implemented as XUL overlays in torbutton.
Finally, for extra safety, we could add an extra C++ patch that ensures that whenever an nsPluginsDir::LoadPlugin implementation is called, the plugin.disable
pref is checked and, if it is true, the function loads nothing and returns an error code. I think such a patch might be upstreamable.