[tor-commits] [torbutton/master] Bug 19273: Avoid JavaScript patching of the external app helper dialog.

gk at torproject.org gk at torproject.org
Sun Jul 17 14:02:14 UTC 2016


commit 6d5dc7845579ca99d9c46d921876e477ba60935f
Author: Kathy Brade <brade at pearlcrescent.com>
Date:   Wed Jun 29 10:33:17 2016 -0400

    Bug 19273: Avoid JavaScript patching of the external app helper dialog.
    
    Display the external app confirmation dialog in response to the new
    "external-app-requested" observer service notification. Remove
    messy overrides of Mozilla components and console log filtering.
    
    Remove obsolete "on-modify-drag-list" observer and pre-Firefox 4.0
    module registration code from the External App Handler component.
---
 src/chrome.manifest                    |   5 +-
 src/chrome/content/torbutton.js        |  93 -----------
 src/components/external-app-blocker.js | 295 +++++----------------------------
 3 files changed, 44 insertions(+), 349 deletions(-)

diff --git a/src/chrome.manifest b/src/chrome.manifest
index e85a205..bb6bcf9 100644
--- a/src/chrome.manifest
+++ b/src/chrome.manifest
@@ -143,9 +143,7 @@ style chrome://global/content/customizeToolbar.xul chrome://torbutton/skin/torbu
 
 # Firefox 4-style component registration
 component {3da0269f-fc29-4e9e-a678-c3b1cafcf13f} components/external-app-blocker.js
-contract @mozilla.org/mime;1 {3da0269f-fc29-4e9e-a678-c3b1cafcf13f}
-contract @mozilla.org/uriloader/external-protocol-service;1 {3da0269f-fc29-4e9e-a678-c3b1cafcf13f}
-contract @mozilla.org/uriloader/external-helper-app-service;1 {3da0269f-fc29-4e9e-a678-c3b1cafcf13f}
+contract @torproject.org/torbutton-extAppBlockerService;1 {3da0269f-fc29-4e9e-a678-c3b1cafcf13f}
 
 component {06322def-6fde-4c06-aef6-47ae8e799629} components/startup-observer.js
 contract @torproject.org/startup-observer;1 {06322def-6fde-4c06-aef6-47ae8e799629}
@@ -173,3 +171,4 @@ contract @torproject.org/torRefSpoofer;1 {65be2be0-ceb4-44c2-91a5-9c75c53430bf}
 category profile-after-change RefSpoofer @torproject.org/torRefSpoofer;1
 category profile-after-change StartupObserver @torproject.org/startup-observer;1
 category profile-after-change DomainIsolator @torproject.org/domain-isolator;1
+category profile-after-change ExtAppBlockerService @torproject.org/torbutton-extAppBlockerService;1
diff --git a/src/chrome/content/torbutton.js b/src/chrome/content/torbutton.js
index 3203cef..5eb26d9 100644
--- a/src/chrome/content/torbutton.js
+++ b/src/chrome/content/torbutton.js
@@ -3054,104 +3054,12 @@ function torbutton_is_windowed(wind) {
     return true;
 }
 
-// This is the console observer used for getting unwanted error messages
-// resulting from JS -> C++ transition filtered out.
-var torbutton_console_observer = {
-
-  obs : null,
-
-  register: function() {
-    this.obs = Cc["@mozilla.org/observer-service;1"].
-      getService(Ci.nsIObserverService);
-    this.obs.addObserver(this, "web-console-created", false);
-  },
-
-  unregister: function() {
-    if (this.obs) {
-      this.obs.removeObserver(this, "web-console-created");
-    }
-  },
-
-  observe: function(subject, topic, data) {
-    if (topic === "web-console-created") {
-      var id = subject.QueryInterface(Ci.nsISupportsString).toString(),
-          con = HUDService.getHudReferenceById(id);
-      con.ui.reportPageErrorOld = con.ui.reportPageError;
-      // Filtering the messages by making them hidden adding the
-      // "hidden-message" class. If the message does not need to get filtered
-      // the original method is executed without any modifications.
-      con.ui.reportPageError =
-        function WCF_reportPageError(aCategory, aScriptError) {
-          var message = aScriptError.errorMessage;
-          if (message && message.indexOf("NS_ERROR_NOT_AVAILABLE") > -1 &&
-              message.indexOf("external-app-blocker.js") > -1) {
-            return this.reportPageErrorOld(aCategory, aScriptError).classList.
-              add("hidden-message");
-          } else {
-            return this.reportPageErrorOld(aCategory, aScriptError);
-          }
-        }
-    }
-  }
-};
-
-// Ideally, we only need to patch/override one method to avoid errors showing up
-// in the browser console. Alas, that is not as easy given the presence of
-// cached messages and the Web Console which we need to consider as well while
-// overriding Devtool methods. Thus, we patch the code path that is called when
-// the browser console is already open AND additionally the one when cached
-// messages are displayed.
-function torbutton_handle_console() {
-  torbutton_console_observer.register();
-  try {
-    // Filtering using the "web-console-created" notification is not enough as
-    // the cached messages are already loaded when it is fired. Therefore,
-    // change |getCachedMessages()| slighty to fit the needs at hand.
-    // The original code is https://mxr.mozilla.org/mozilla-esr24/source/
-    // toolkit/devtools/webconsole/WebConsoleUtils.jsm#998 ff. and distributed
-    // under the MPL 2.0 license.
-    ConsoleServiceListener.prototype.getCachedMessages =
-      function CSL_getCachedMessages(aIncludePrivate = false) {
-        var innerWindowID = this.window ? WebConsoleUtils.
-          getInnerWindowId(this.window) : null;
-        var errors = Services.console.getMessageArray() || [];
-
-        return errors.filter((aError) => {
-          if (aError instanceof Ci.nsIScriptError) {
-            var message = aError.message;
-            if (message && message.indexOf("NS_ERROR_NOT_AVAILABLE") > -1 &&
-                message.indexOf("external-app-blocker.js") > -1) {
-              return false;
-            }
-            if (!aIncludePrivate && aError.isFromPrivateWindow) {
-              return false;
-            }
-            if (innerWindowID &&
-                (aError.innerWindowID != innerWindowID ||
-                 !this.isCategoryAllowed(aError.category))) {
-              return false;
-            }
-          }
-          else if (innerWindowID) {
-            // If this is not an nsIScriptError and we need to do window-based
-            // filtering we skip this message.
-            return false;
-          }
-
-          return true;
-        });
-      };
-  } catch (e) {}
-}
-
 // Bug 1506 P3: This is needed pretty much only for the version check
 // and the window resizing. See comments for individual functions for
 // details
 function torbutton_new_window(event)
 {
     torbutton_log(3, "New window");
-    // Working around #9901, sigh...
-    torbutton_handle_console();
     var browser = getBrowser();
 
     if(!browser) {
@@ -3199,7 +3107,6 @@ function torbutton_new_window(event)
 function torbutton_close_window(event) {
     torbutton_window_pref_observer.unregister();
     torbutton_tor_check_observer.unregister();
-    torbutton_console_observer.unregister();
 
     window.removeEventListener("sizemodechange", m_tb_resize_handler,
         false);
diff --git a/src/components/external-app-blocker.js b/src/components/external-app-blocker.js
index b349ab4..7b22a90 100644
--- a/src/components/external-app-blocker.js
+++ b/src/components/external-app-blocker.js
@@ -6,165 +6,78 @@
  * External App Handler.
  * Handles displaying confirmation dialogs for external apps and protocols
  * due to Firefox Bug https://bugzilla.mozilla.org/show_bug.cgi?id=440892
+ *
+ * Also implements an observer that filters drag events to prevent OS
+ * access to URLs (a potential proxy bypass vector).
  *************************************************************************/
 
-// Module specific constants
-const kMODULE_NAME = "Torbutton External App Handler";
-
-const kMODULE_CONTRACTID_APP = "@mozilla.org/uriloader/external-helper-app-service;1";
-const kMODULE_CONTRACTID_PROTO = "@mozilla.org/uriloader/external-protocol-service;1";
-const kMODULE_CONTRACTID_MIME = "@mozilla.org/mime;1";
-
-
-const kMODULE_CID = Components.ID("3da0269f-fc29-4e9e-a678-c3b1cafcf13f");
-
-/* Mozilla defined interfaces for FF3.0 */
-const kREAL_EXTERNAL_CID = "{A7F800E0-4306-11d4-98D0-001083010E9B}";
-const kExternalInterfaces = ["nsIObserver", "nsIMIMEService",
-                             "nsIExternalHelperAppService",
-                             "nsISupportsWeakReference", // XXX: Uh-oh...
-                             "nsIExternalProtocolService",
-                             "nsPIExternalAppLauncher"];
-                             
-const Cr = Components.results;
 const Cc = Components.classes;
 const Ci = Components.interfaces;
+const Cu = Components.utils;
 
-var appInfo = Components.classes["@mozilla.org/xre/app-info;1"]
-                .getService(Components.interfaces.nsIXULAppInfo);
-var versionChecker = Components.classes["@mozilla.org/xpcom/version-comparator;1"]
-                       .getService(Components.interfaces.nsIVersionComparator);
-var is_ff3 = (versionChecker.compare(appInfo.version, "3.0a1") >= 0);
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
-function ExternalWrapper() {
-  this.logger = Components.classes["@torproject.org/torbutton-logger;1"]
-      .getService(Components.interfaces.nsISupports).wrappedJSObject;
-  this.logger.log(3, "Component Load 0: New ExternalWrapper.");
+// Module specific constants
+const kMODULE_NAME = "Torbutton External App Handler";
+const kCONTRACT_ID = "@torproject.org/torbutton-extAppBlockerService;1";
+const kMODULE_CID = Components.ID("3da0269f-fc29-4e9e-a678-c3b1cafcf13f");
 
-  this._real_external = Components.classesByID[kREAL_EXTERNAL_CID];
-  this._interfaces = kExternalInterfaces;
+const kInterfaces = [Ci.nsIObserver, Ci.nsIClassInfo];
 
-  this._prefs = Components.classes["@mozilla.org/preferences-service;1"]
-      .getService(Components.interfaces.nsIPrefBranch);
+function ExternalAppBlocker() {
+  this.logger = Cc["@torproject.org/torbutton-logger;1"]
+      .getService(Ci.nsISupports).wrappedJSObject;
+  this.logger.log(3, "Component Load 0: New ExternalAppBlocker.");
 
-  this._external = function() {
-    var external = this._real_external.getService();
-    for (var i = 0; i < this._interfaces.length; i++) {
-      external.QueryInterface(Components.interfaces[this._interfaces[i]]);
-    }
-    return external;
-  };
-    
-  this.copyMethods(this._external());
+  this._prefs = Cc["@mozilla.org/preferences-service;1"]
+      .getService(Ci.nsIPrefBranch);
 
   try {
     var observerService = Cc["@mozilla.org/observer-service;1"].
         getService(Ci.nsIObserverService);
-    observerService.addObserver(this, "on-modify-drag-list", false);
+    observerService.addObserver(this, "external-app-requested", false);
     observerService.addObserver(this, "on-datatransfer-available", false);
   } catch(e) {
-    this.logger.log(5, "Failed to register drag observer");
+    this.logger.log(5, "Failed to register external app observer or drag observer");
   }
 }
 
-ExternalWrapper.prototype =
+ExternalAppBlocker.prototype =
 {
-  QueryInterface: function(iid) {
-    if (iid.equals(Components.interfaces.nsIClassInfo)
-        || iid.equals(Components.interfaces.nsISupports)) {
-      return this;
-    }
-
-    /* We perform this explicit check first because otherwise
-     * the JSD exception logs are full of noise */
-    var external = this._external().QueryInterface(iid);
-    this.copyMethods(external);
-
-    return this;
-  },
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports, Ci.nsIObserver]),
 
   // make this an nsIClassInfo object
-  flags: Components.interfaces.nsIClassInfo.DOM_OBJECT,
-
-  // method of nsIClassInfo
-
-  classDescription: "@mozilla.org/uriloader/external-helper-app-service;1",
-  contractID: "@mozilla.org/uriloader/external-helper-app-service;1",
+  flags: Ci.nsIClassInfo.DOM_OBJECT,
+  classDescription: kMODULE_NAME,
+  contractID: kCONTRACT_ID,
   classID: kMODULE_CID,
 
   // method of nsIClassInfo
   getInterfaces: function(count) {
-    var interfaceList = [Components.interfaces.nsIClassInfo];
-    for (var i = 0; i < this._interfaces.length; i++) {
-      interfaceList.push(Components.interfaces[this._interfaces[i]]);
-    }
-
-    count.value = interfaceList.length;
-    return interfaceList;
+    count.value = kInterfaces.length;
+    return kInterfaces;
   },
 
   // method of nsIClassInfo  
   getHelperForLanguage: function(count) { return null; },
 
   /* Determine whether we should ask the user to run the app */
-  blockApp: function() {
+  _blockApp: function() {
     return this._prefs.getBoolPref("extensions.torbutton.tor_enabled");
   },
 
-  /* Copies methods from the true object we are wrapping */
-  copyMethods: function(wrapped) {
-    var mimic = function(newObj, method) {
-       if(typeof(wrapped[method]) == "function") {
-          // Code courtesy of timeless: 
-          // http://www.webwizardry.net/~timeless/windowStubs.js
-          var params = [];
-          params.length = wrapped[method].length;
-          var x = 0;
-          var call;
-          if(params.length) call = "("+params.join().replace(/(?:)/g,function(){return "p"+(++x)})+")";
-          else call = "()";
-
-          var fun = "(function "+call+"{"+
-            "if (arguments.length < "+wrapped[method].length+")"+
-            "  throw Components.results.NS_ERROR_XPC_NOT_ENOUGH_ARGS;"+
-            "return wrapped."+method+".apply(wrapped, arguments);})";
-          newObj[method] = eval(fun);
-       } else {
-          newObj.__defineGetter__(method, function() { return wrapped[method]; });
-          newObj.__defineSetter__(method, function(val) { wrapped[method] = val; });
-      }
-    };
-    for (var method in wrapped) {
-      if(typeof(this[method]) == "undefined") mimic(this, method);
-    }
-  },
-
-  loadURI: function(aUri, aContext) {
-    if(this.blockApp()) {
-      var check = {value: false};
-      var result = this._confirmLaunch(aUri.spec, check);
-
-      if (result != 0) {
-        return null;
-      }
-    }
- 
-    return this._external().loadURI(aUri, aContext);
-  },
-
-  // loadUrl calls loadURI
-
-  _confirmLaunch: function(urispec, check) {
+  // Returns true if launch should proceed.
+  _confirmLaunch: function() {
     if (!this._prefs.getBoolPref("extensions.torbutton.launch_warning")) {
-      return 0;
+      return true;
     }
 
     var wm = Cc["@mozilla.org/appshell/window-mediator;1"]
-               .getService(Components.interfaces.nsIWindowMediator);
+               .getService(Ci.nsIWindowMediator);
     var chrome = wm.getMostRecentWindow("navigator:browser");
 
-    var prompts = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
-                            .getService(Components.interfaces.nsIPromptService);
+    var prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"]
+                            .getService(Ci.nsIPromptService);
     var flags = prompts.BUTTON_POS_0 * prompts.BUTTON_TITLE_IS_STRING +
                 prompts.BUTTON_POS_1 * prompts.BUTTON_TITLE_IS_STRING +
                 prompts.BUTTON_DELAY_ENABLE +
@@ -178,77 +91,32 @@ ExternalWrapper.prototype =
     var cancel = chrome.torbutton_get_property_string("torbutton.popup.cancel");
     var dontask = chrome.torbutton_get_property_string("torbutton.popup.dontask");
 
+    var check = {value: false};
     var result = prompts.confirmEx(chrome, title, app+note+suggest+" ",
                                    flags, launch, cancel, "", dontask, check);
 
-    //var result = prompts.confirmEx(chrome, title, app+urispec+note+suggest+" ",
-    //                               flags, launch, cancel, "", dontask, check);
-
     if (check.value) {
       this._prefs.setBoolPref("extensions.torbutton.launch_warning", false);
     }
 
-    return result;
+    return (0 == result);
   },
   
-  doContent: function(aMimeContentType, aRequest, aContentContext,
-                      aForceSave, aWindowContext) {
-    if(this.blockApp()) {
-      var check = {value: false};
-      var result = this._confirmLaunch(aRequest.name, check);
-
-      if (result != 0) {
-        return null;
-      }
-    }
- 
-    return this._external().doContent(aMimeContentType, aRequest,
-                                 aContentContext, aForceSave, aWindowContext);
-  },
-
   observe: function(subject, topic, data) {
-    // XXX: The on-modify-drag-list is TBB specific and can be removed.
-    // FF31 added the on-datatransfer-available observer instead.
-    if (topic == "on-modify-drag-list") {
-      this.logger.log(3, "Got drag observer event");
-      try {
-        subject.QueryInterface(Ci.nsISupportsArray);
-      } catch(e) {
-        this.logger.log(5, "Drag and Drop subject is not an array: "+e);
+    if (topic == "external-app-requested") {
+      this.logger.log(3, "External app requested");
+      // subject.data is true if the launch should be canceled.
+      if (this._blockApp() && (subject instanceof Ci.nsISupportsPRBool)
+          && !subject.data /* not canceled already */
+          && !this._confirmLaunch()) {
+        subject.data = true; // The user said to cancel the launch.
       }
-
-      return this.filterDragURLs(subject);
     } else if (topic == "on-datatransfer-available") {
       this.logger.log(3, "The DataTransfer is available");
       return this.filterDataTransferURLs(subject);
     }
   },
 
-  filterDragURLs: function(aTransferableArray) {
-    for(var i = 0; i < aTransferableArray.Count(); i++) {
-      this.logger.log(3, "Inspecting drag+drop transfer: "+i);
-      var tr = aTransferableArray.GetElementAt(i);
-      tr.QueryInterface(Ci.nsITransferable);
-
-      var flavors = tr.flavorsTransferableCanExport()
-                      .QueryInterface(Ci.nsISupportsArray);
-
-      for (var f=0; f < flavors.Count(); f++) {
-        var flavor =flavors.GetElementAt(f);
-        flavor.QueryInterface(Ci.nsISupportsCString);
-
-        this.logger.log(3, "Got drag+drop flavor: "+flavor);
-        if (flavor == "text/x-moz-url" ||
-            flavor == "text/x-moz-url-data" ||
-            flavor == "text/uri-list" ||
-            flavor == "application/x-moz-file-promise-url") {
-          this.logger.log(3, "Removing "+flavor);
-          try { tr.removeDataFlavor(flavor); } catch(e) {}
-        }
-      }
-    }
-  },
-
   filterDataTransferURLs: function(aDataTransfer) {
     var types = null;
     var type = "";
@@ -274,83 +142,4 @@ ExternalWrapper.prototype =
 
 };
 
-var ExternalWrapperSingleton = null;
-var ExternalWrapperFactory = new Object();
-
-ExternalWrapperFactory.createInstance = function (outer, iid)
-{
-  if (outer != null) {
-    Components.returnCode = Cr.NS_ERROR_NO_AGGREGATION;
-    return null;
-  }
-
-  if(!ExternalWrapperSingleton)
-    ExternalWrapperSingleton = new ExternalWrapper();
-
-  return ExternalWrapperSingleton;
-};
-
-
-/**
- * JS XPCOM component registration goop:
- *
- * Everything below is boring boilerplate and can probably be ignored.
- */
-
-var ExternalWrapperModule = new Object();
-
-ExternalWrapperModule.registerSelf = 
-function (compMgr, fileSpec, location, type) {
-  var nsIComponentRegistrar = Components.interfaces.nsIComponentRegistrar;
-  compMgr = compMgr.QueryInterface(nsIComponentRegistrar);
-  compMgr.registerFactoryLocation(kMODULE_CID,
-                                  kMODULE_NAME,
-                                  kMODULE_CONTRACTID_APP,
-                                  fileSpec,
-                                  location,
-                                  type);
-
-  compMgr.registerFactoryLocation(kMODULE_CID,
-                                  kMODULE_NAME,
-                                  kMODULE_CONTRACTID_PROTO,
-                                  fileSpec,
-                                  location,
-                                  type);
-
-  compMgr.registerFactoryLocation(kMODULE_CID,
-                                  kMODULE_NAME,
-                                  kMODULE_CONTRACTID_MIME,
-                                  fileSpec,
-                                  location,
-                                  type);
-
-};
-
-ExternalWrapperModule.getClassObject = function (compMgr, cid, iid)
-{
-  if (cid.equals(kMODULE_CID))
-    return ExternalWrapperFactory;
-
-  Components.returnCode = Cr.NS_ERROR_NOT_REGISTERED;
-  return null;
-};
-
-ExternalWrapperModule.canUnload = function (compMgr)
-{
-  return true;
-};
-
-/**
-* XPCOMUtils.generateNSGetFactory was introduced in Mozilla 2 (Firefox 4).
-* XPCOMUtils.generateNSGetModule is for Mozilla 1.9.2 (Firefox 3.6).
-*/
-Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
-if (XPCOMUtils.generateNSGetFactory) {
-    var NSGetFactory = XPCOMUtils.generateNSGetFactory([ExternalWrapper]);
-} else {
-    function NSGetModule(compMgr, fileSpec)
-    {
-      return ExternalWrapperModule;
-    }
-}
-
+var NSGetFactory = XPCOMUtils.generateNSGetFactory([ExternalAppBlocker]);





More information about the tor-commits mailing list