Bug 777196 - Prevent non-chrome processes from accessing the content preferences. r=mak
☠☠ backed out by ff4aca0cf3ce ☠ ☠
authorGabriele Svelto <gsvelto@mozilla.com>
Tue, 30 Jul 2013 09:58:44 +0200
changeset 153379 7ef4465f20bdf42285e4fa8f8ede9bb6ee6dd817
parent 153378 a850d7025a3c1343f4837e9673c3deb767c2f6f8
child 153380 786f02cdbc120dce21f174051a4b89491d6ee906
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs777196
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 777196 - Prevent non-chrome processes from accessing the content preferences. r=mak CLOSED TREE
toolkit/components/contentprefs/ContentPrefService2.jsm
toolkit/components/contentprefs/nsContentPrefService.js
toolkit/components/contentprefs/tests/moz.build
toolkit/components/contentprefs/tests/unit_ipc/contentPrefs_childipc.js
toolkit/components/contentprefs/tests/unit_ipc/head_contentPrefs.js
toolkit/components/contentprefs/tests/unit_ipc/tail_contentPrefs.js
toolkit/components/contentprefs/tests/unit_ipc/test_contentPrefs_parentipc.js
toolkit/components/contentprefs/tests/unit_ipc/xpcshell.ini
--- a/toolkit/components/contentprefs/ContentPrefService2.jsm
+++ b/toolkit/components/contentprefs/ContentPrefService2.jsm
@@ -191,17 +191,17 @@ ContentPrefService2.prototype = {
     checkNameArg(name);
     checkValueArg(value);
     checkCallbackArg(callback, false);
 
     if (context && context.usePrivateBrowsing) {
       this._pbStore.set(group, name, value);
       this._schedule(function () {
         cbHandleCompletion(callback, Ci.nsIContentPrefCallback2.COMPLETE_OK);
-        this._cps._broadcastPrefSet(group, name, value);
+        this._cps._notifyPrefSet(group, name, value);
       });
       return;
     }
 
     // Invalidate the cached value so consumers accessing the cache between now
     // and when the operation finishes don't get old data.
     this._cache.remove(group, name);
 
@@ -261,17 +261,17 @@ ContentPrefService2.prototype = {
     stmts.push(stmt);
 
     this._execStmts(stmts, {
       onDone: function onDone(reason, ok) {
         if (ok)
           this._cache.setWithCast(group, name, value);
         cbHandleCompletion(callback, reason);
         if (ok)
-          this._cps._broadcastPrefSet(group, name, value);
+          this._cps._notifyPrefSet(group, name, value);
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
       }
     });
   },
 
   removeByDomainAndName: function CPS2_removeByDomainAndName(group, name,
@@ -351,17 +351,17 @@ ContentPrefService2.prototype = {
               prefs.set(sgroup, name, undefined);
               this._pbStore.remove(sgroup, name);
             }
           }
         }
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, , ] in prefs) {
-            this._cps._broadcastPrefRemoved(sgroup, name);
+            this._cps._notifyPrefRemoved(sgroup, name);
           }
         }
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
       }
     });
   },
@@ -443,17 +443,17 @@ ContentPrefService2.prototype = {
           for (let [sgroup, sname, ] in this._pbStore) {
             prefs.set(sgroup, sname, undefined);
             this._pbStore.remove(sgroup, sname);
           }
         }
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, sname, ] in prefs) {
-            this._cps._broadcastPrefRemoved(sgroup, sname);
+            this._cps._notifyPrefRemoved(sgroup, sname);
           }
         }
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
       }
     });
   },
@@ -500,17 +500,17 @@ ContentPrefService2.prototype = {
           for (let [sgroup, sname, ] in this._pbStore) {
             prefs.set(sgroup, sname, undefined);
           }
           this._pbStore.removeAllGroups();
         }
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, sname, ] in prefs) {
-            this._cps._broadcastPrefRemoved(sgroup, sname);
+            this._cps._notifyPrefRemoved(sgroup, sname);
           }
         }
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
       }
     });
   },
@@ -580,17 +580,17 @@ ContentPrefService2.prototype = {
               prefs.set(sgroup, name, undefined);
               this._pbStore.remove(sgroup, name);
             }
           }
         }
         cbHandleCompletion(callback, reason);
         if (ok) {
           for (let [sgroup, , ] in prefs) {
-            this._cps._broadcastPrefRemoved(sgroup, name);
+            this._cps._notifyPrefRemoved(sgroup, name);
           }
         }
       },
       onError: function onError(nsresult) {
         cbHandleError(callback, nsresult);
       }
     });
   },
--- a/toolkit/components/contentprefs/nsContentPrefService.js
+++ b/toolkit/components/contentprefs/nsContentPrefService.js
@@ -4,117 +4,34 @@
 
 const Ci = Components.interfaces;
 const Cc = Components.classes;
 const Cr = Components.results;
 const Cu = Components.utils;
 
 const CACHE_MAX_GROUP_ENTRIES = 100;
 
-
-// We have a whitelist for getting/setting. This is because
-// there are potential privacy issues with a compromised
-// content process checking the user's content preferences
-// and using that to discover all the websites visited, etc.
-// Also there are both potential race conditions (if two processes
-// set more than one value in succession, and the values
-// only make sense together), as well as security issues, if
-// a compromised content process can send arbitrary setPref
-// messages. The whitelist contains only those settings that
-// are not at risk for either.
-// We currently whitelist saving/reading the last directory of file
-// uploads, and the last current spellchecker dictionary which are so far
-// the only need we have identified.
-const REMOTE_WHITELIST = [
-  "browser.upload.lastDir",
-  "spellcheck.lang",
-];
-
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 /**
  * Remotes the service. All the remoting/electrolysis code is in here,
  * so the regular service code below remains uncluttered and maintainable.
  */
 function electrolify(service) {
   // FIXME: For now, use the wrappedJSObject hack, until bug
   //        593407 which will clean that up.
   //        Note that we also use this in the xpcshell tests, separately.
   service.wrappedJSObject = service;
 
   var appInfo = Cc["@mozilla.org/xre/app-info;1"];
-  if (!appInfo || appInfo.getService(Ci.nsIXULRuntime).processType ==
-      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
-    // Parent process
-
-    service.messageManager = Cc["@mozilla.org/parentprocessmessagemanager;1"].
-                             getService(Ci.nsIMessageBroadcaster);
-
-    // Setup listener for child messages. We don't need to call
-    // addMessageListener as the wakeup service will do that for us.
-    service.receiveMessage = function(aMessage) {
-      var json = aMessage.json;
-
-      if (REMOTE_WHITELIST.indexOf(json.name) == -1)
-        return { succeeded: false };
-
-      switch (aMessage.name) {
-        case "ContentPref:getPref":
-          return { succeeded: true,
-                   value: service.getPref(json.group, json.name, json.value) };
-
-        case "ContentPref:setPref":
-          service.setPref(json.group, json.name, json.value);
-          return { succeeded: true };
-      }
-    };
-  } else {
+  if (appInfo && appInfo.getService(Ci.nsIXULRuntime).processType !=
+      Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT)
+  {
     // Child process
-
     service._dbInit = function(){}; // No local DB
-
-    service.messageManager = Cc["@mozilla.org/childprocessmessagemanager;1"].
-                             getService(Ci.nsISyncMessageSender);
-
-    // Child method remoting
-    [
-      ['getPref', ['group', 'name'], ['_parseGroupParam']],
-      ['setPref', ['group', 'name', 'value'], ['_parseGroupParam']],
-    ].forEach(function(data) {
-      var method = data[0];
-      var params = data[1];
-      var parsers = data[2];
-      service[method] = function __remoted__() {
-        var json = {};
-        for (var i = 0; i < params.length; i++) {
-          if (params[i]) {
-            json[params[i]] = arguments[i];
-            if (parsers[i])
-              json[params[i]] = this[parsers[i]](json[params[i]]);
-          }
-        }
-        var ret = service.messageManager.sendSyncMessage('ContentPref:' + method, json)[0];
-        if (!ret.succeeded)
-          throw "ContentPrefs remoting failed to pass whitelist";
-        return ret.value;
-      };
-    });
-
-    // Listen to preference change notifications from the parent and notify
-    // observers in the child process according to the change
-    service.messageManager.addMessageListener("ContentPref:notifyPrefSet",
-      function(aMessage) {
-        var json = aMessage.json;
-        service._notifyPrefSet(json.group, json.name, json.value);
-      });
-    service.messageManager.addMessageListener("ContentPref:notifyPrefRemoved",
-      function(aMessage) {
-        var json = aMessage.json;
-        service._notifyPrefRemoved(json.group, json.name);
-      });
   }
 }
 
 function ContentPrefService() {
   electrolify(this);
 
   // If this throws an exception, it causes the getService call to fail,
   // but the next time a consumer tries to retrieve the service, we'll try
@@ -345,17 +262,17 @@ ContentPrefService.prototype = {
       if (currentValue == aValue)
         return;
     }
 
     var group = this._parseGroupParam(aGroup);
 
     if (aContext && aContext.usePrivateBrowsing) {
       this._privModeStorage.setWithCast(group, aName, aValue);
-      this._broadcastPrefSet(group, aName, aValue);
+      this._notifyPrefSet(group, aName, aValue);
       return;
     }
 
     var settingID = this._selectSettingID(aName) || this._insertSetting(aName);
     var groupID, prefID;
     if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
@@ -367,17 +284,17 @@ ContentPrefService.prototype = {
 
     // Update the existing record, if any, or create a new one.
     if (prefID)
       this._updatePref(prefID, aValue);
     else
       this._insertPref(groupID, settingID, aValue);
 
     this._cache.setWithCast(group, aName, aValue);
-    this._broadcastPrefSet(group, aName, aValue);
+    this._notifyPrefSet(group, aName, aValue);
   },
 
   hasPref: function ContentPrefService_hasPref(aGroup, aName, aContext) {
     // XXX If consumers end up calling this method regularly, then we should
     // optimize this to query the database directly.
     return (typeof this.getPref(aGroup, aName, aContext) != "undefined");
   },
 
@@ -395,17 +312,17 @@ ContentPrefService.prototype = {
     // If there's no old value, then there's nothing to remove.
     if (!this.hasPref(aGroup, aName, aContext))
       return;
 
     var group = this._parseGroupParam(aGroup);
 
     if (aContext && aContext.usePrivateBrowsing) {
       this._privModeStorage.remove(group, aName);
-      this._broadcastPrefRemoved(group, aName);
+      this._notifyPrefRemoved(group, aName);
       return;
     }
 
     var settingID = this._selectSettingID(aName);
     var groupID, prefID;
     if (group == null) {
       groupID = null;
       prefID = this._selectGlobalPrefID(settingID);
@@ -418,17 +335,17 @@ ContentPrefService.prototype = {
     this._deletePref(prefID);
 
     // Get rid of extraneous records that are no longer being used.
     this._deleteSettingIfUnused(settingID);
     if (groupID)
       this._deleteGroupIfUnused(groupID);
 
     this._cache.remove(group, aName);
-    this._broadcastPrefRemoved(group, aName);
+    this._notifyPrefRemoved(group, aName);
   },
 
   removeGroupedPrefs: function ContentPrefService_removeGroupedPrefs(aContext) {
     // will not delete global preferences
     if (aContext && aContext.usePrivateBrowsing) {
         // keep only global prefs
         this._privModeStorage.removeAllGroups();
     }
@@ -453,17 +370,17 @@ ContentPrefService.prototype = {
     if (!aName)
       throw Components.Exception("aName cannot be null or an empty string",
                                  Cr.NS_ERROR_ILLEGAL_VALUE);
 
     if (aContext && aContext.usePrivateBrowsing) {
       for (let [group, name, ] in this._privModeStorage) {
         if (name === aName) {
           this._privModeStorage.remove(group, aName);
-          this._broadcastPrefRemoved(group, aName);
+          this._notifyPrefRemoved(group, aName);
         }
       }
     }
 
     var settingID = this._selectSettingID(aName);
     if (!settingID)
       return;
     
@@ -495,17 +412,17 @@ ContentPrefService.prototype = {
     this._dbConnection.executeSimpleSQL("DELETE FROM prefs WHERE settingID = " + settingID);
     this._dbConnection.executeSimpleSQL("DELETE FROM settings WHERE id = " + settingID);
 
     for (var i = 0; i < groupNames.length; i++) {
       this._cache.remove(groupNames[i], aName);
       if (groupNames[i]) // ie. not null, which will be last (and i == groupIDs.length)
         this._deleteGroupIfUnused(groupIDs[i]);
       if (!aContext || !aContext.usePrivateBrowsing) {
-        this._broadcastPrefRemoved(groupNames[i], aName);
+        this._notifyPrefRemoved(groupNames[i], aName);
       }
     }
   },
 
   getPrefs: function ContentPrefService_getPrefs(aGroup, aContext) {
     var group = this._parseGroupParam(aGroup);
     if (aContext && aContext.usePrivateBrowsing) {
         let prefs = Cc["@mozilla.org/hash-property-bag;1"].
@@ -614,48 +531,16 @@ ContentPrefService.prototype = {
         observer.onContentPrefSet(aGroup, aName, aValue);
       }
       catch(ex) {
         Cu.reportError(ex);
       }
     }
   },
 
-  /**
-   * Notify all observers in the current process about the removal of a
-   * preference and send a message to all other processes so that they can in
-   * turn notify their observers about the change. This is meant to be called
-   * only in the parent process. Only whitelisted preferences are broadcast to
-   * the child processes.
-   */
-  _broadcastPrefRemoved: function ContentPrefService__broadcastPrefRemoved(aGroup, aName) {
-    this._notifyPrefRemoved(aGroup, aName);
-
-    if (REMOTE_WHITELIST.indexOf(aName) != -1) {
-      this.messageManager.broadcastAsyncMessage('ContentPref:notifyPrefRemoved',
-        { "group": aGroup, "name": aName } );
-    }
-  },
-
-  /**
-   * Notify all observers in the current process about a preference change and
-   * send a message to all other processes so that they can in turn notify
-   * their observers about the change. This is meant to be called only in the
-   * parent process. Only whitelisted preferences are broadcast to the child
-   * processes.
-   */
-  _broadcastPrefSet: function ContentPrefService__broadcastPrefSet(aGroup, aName, aValue) {
-    this._notifyPrefSet(aGroup, aName, aValue);
-
-    if (REMOTE_WHITELIST.indexOf(aName) != -1) {
-      this.messageManager.broadcastAsyncMessage('ContentPref:notifyPrefSet',
-        { "group": aGroup, "name": aName, "value": aValue } );
-    }
-  },
-
   _grouper: null,
   get grouper() {
     if (!this._grouper)
       this._grouper = Cc["@mozilla.org/content-pref/hostname-grouper;1"].
                       getService(Ci.nsIContentURIGrouper);
     return this._grouper;
   },
 
--- a/toolkit/components/contentprefs/tests/moz.build
+++ b/toolkit/components/contentprefs/tests/moz.build
@@ -2,12 +2,8 @@
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 MODULE = 'test_toolkit_contentprefs'
 
 XPCSHELL_TESTS_MANIFESTS += ['unit/xpcshell.ini', 'unit_cps2/xpcshell.ini']
-
-# FIXME/bug 575918: out-of-process xpcshell is broken on OS X
-if CONFIG['OS_ARCH'] != 'Darwin':
-    XPCSHELL_TESTS_MANIFESTS += ['unit_ipc/xpcshell.ini']
deleted file mode 100644
--- a/toolkit/components/contentprefs/tests/unit_ipc/contentPrefs_childipc.js
+++ /dev/null
@@ -1,39 +0,0 @@
-
-function run_test() {
-  do_check_true(inChildProcess(), "test harness should never call us directly");
-
-  var cps = Cc["@mozilla.org/content-pref/service;1"].
-            createInstance(Ci.nsIContentPrefService);
-
-  // Cannot get general values
-  try {
-    cps.getPref("group", "name")
-    do_check_false(true, "Must have thrown exception on getting general value");
-  }
-  catch(e) { }
-
-  // Cannot set general values
-  try {
-    cps.setPref("group", "name", "someValue2");
-    do_check_false(true, "Must have thrown exception on setting general value");
-  }
-  catch(e) { }
-
-  // Can set&get whitelisted values
-  cps.setPref("group", "browser.upload.lastDir", "childValue", null);
-  do_check_eq(cps.getPref("group", "browser.upload.lastDir", null), "childValue");
-
-  // Test sending URI
-  var ioSvc = Cc["@mozilla.org/network/io-service;1"].
-              getService(Ci.nsIIOService);
-  var uri = ioSvc.newURI("http://mozilla.org", null, null);
-  cps.setPref(uri, "browser.upload.lastDir", "childValue2", null);
-  do_check_eq(cps.getPref(uri, "browser.upload.lastDir", null), "childValue2");
-
-  // Previous value
-  do_check_eq(cps.getPref("group", "browser.upload.lastDir", null), "childValue");
-
-  // Tell parent to finish and clean up
-  cps.wrappedJSObject.messageManager.sendSyncMessage('ContentPref:QUIT');
-}
-
deleted file mode 100644
--- a/toolkit/components/contentprefs/tests/unit_ipc/head_contentPrefs.js
+++ /dev/null
@@ -1,5 +0,0 @@
-// initializing profile here because do_get_profile cannot be called
-// from a content process
-do_get_profile();
-
-load("../unit/head_contentPrefs.js");
deleted file mode 100644
--- a/toolkit/components/contentprefs/tests/unit_ipc/tail_contentPrefs.js
+++ /dev/null
@@ -1,3 +0,0 @@
-
-load("../unit/tail_contentPrefs.js");
-
deleted file mode 100644
--- a/toolkit/components/contentprefs/tests/unit_ipc/test_contentPrefs_parentipc.js
+++ /dev/null
@@ -1,88 +0,0 @@
-
-function run_test() {
-  // Check received messages
-
-  var cps = Cc["@mozilla.org/content-pref/service;1"].
-            createInstance(Ci.nsIContentPrefService).
-            wrappedJSObject;
-
-  var messageHandler = cps;
-  // FIXME: For now, use the wrappedJSObject hack, until bug
-  //        593407 which will clean that up. After that, use
-  //        the commented out line below it.
-  messageHandler = cps.wrappedJSObject;
-  //messageHandler = cps.QueryInterface(Ci.nsIMessageListener);
-
-  // Cannot get values
-  do_check_false(messageHandler.receiveMessage({
-    name: "ContentPref:getPref",
-    json: { group: 'group2', name: 'name' } }).succeeded);
-
-  // Cannot set general values
-  messageHandler.receiveMessage({ name: "ContentPref:setPref",
-    json: { group: 'group2', name: 'name', value: 'someValue' } });
-  do_check_eq(cps.getPref('group', 'name', null), undefined);
-
-  // Can set whitelisted values
-  do_check_true(messageHandler.receiveMessage({ name: "ContentPref:setPref",
-    json: { group: 'group2', name: 'browser.upload.lastDir',
-            value: 'someValue' } }).succeeded);
-  do_check_eq(cps.getPref('group2', 'browser.upload.lastDir', null), 'someValue');
-
-  // Prepare for child tests
-
-  // Manually listen to messages - the wakeup manager should do this
-  // for us, but it doesn't run in xpcshell tests.
-  var messageProxy = {
-    receiveMessage: function(aMessage) {
-      if (aMessage.name == 'ContentPref:QUIT') {
-        // Undo mock storage.
-        delete cps._mockStorage;
-        delete cps._messageProxy;
-        cps.setPref = cps.old_setPref;
-        cps.getPref = cps.old_getPref;
-        cps._dbInit = cps.old__dbInit;
-        // Unlisten to messages
-        mM.removeMessageListener("ContentPref:setPref", messageProxy);
-        mM.removeMessageListener("ContentPref:getPref", messageProxy);
-        mM.removeMessageListener("ContentPref:QUIT", messageProxy);
-        do_test_finished();
-        return true;
-      } else {
-        return messageHandler.receiveMessage(aMessage);
-      }
-    },
-  };
-
-  var mM = Cc["@mozilla.org/parentprocessmessagemanager;1"].
-           getService(Ci.nsIMessageListenerManager);
-  mM.addMessageListener("ContentPref:setPref", messageProxy);
-  mM.addMessageListener("ContentPref:getPref", messageProxy);
-  mM.addMessageListener("ContentPref:QUIT", messageProxy);
-
-  // Mock storage. This is necessary because
-  // the IPC xpcshell setup doesn't do well with the normal storage
-  // engine.
-
-  cps = cps.wrappedJSObject;
-  cps._mockStorage = {};
-
-  cps.old_setPref = cps.setPref;
-  cps.setPref = function(aGroup, aName, aValue) {
-    this._mockStorage[aGroup+':'+aName] = aValue;
-  }
-
-  cps.old_getPref = cps.getPref;
-  cps.getPref = function(aGroup, aName) {
-    return this._mockStorage[aGroup+':'+aName];
-  }
-
-  cps.old__dbInit = cps._dbInit;
-  cps._dbInit = function(){};
-
-  cps._messageProxy = messageProxy; // keep it alive
-  do_test_pending();
-
-  run_test_in_child("contentPrefs_childipc.js");
-}
-
deleted file mode 100644
--- a/toolkit/components/contentprefs/tests/unit_ipc/xpcshell.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[DEFAULT]
-head = head_contentPrefs.js
-tail = tail_contentPrefs.js
-
-[test_contentPrefs_parentipc.js]