From dcf5365ef2e36595b1550063cd61c5ce481be9da Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 26 Nov 2013 16:31:07 +0000 Subject: [PATCH] Bug 923625 - DataStore sends the principal as argument in sendAsyncMessage, r=ehsan --- dom/datastore/DataStoreService.js | 33 ++++++++++++++-------- dom/datastore/DataStoreServiceInternal.jsm | 11 ++++++-- dom/datastore/tests/test_app_install.html | 3 +- dom/datastore/tests/test_arrays.html | 4 +++ dom/datastore/tests/test_basic.html | 4 +++ dom/datastore/tests/test_bug924104.html | 4 +++ dom/datastore/tests/test_changes.html | 4 +++ dom/datastore/tests/test_oop.html | 4 +++ dom/datastore/tests/test_readonly.html | 3 +- dom/datastore/tests/test_sync.html | 4 +++ dom/ipc/ContentParent.cpp | 9 ++++-- dom/ipc/TabParent.cpp | 9 ++++-- 12 files changed, 69 insertions(+), 23 deletions(-) diff --git a/dom/datastore/DataStoreService.js b/dom/datastore/DataStoreService.js index 0fde3e3ebf8..034cadf5255 100644 --- a/dom/datastore/DataStoreService.js +++ b/dom/datastore/DataStoreService.js @@ -236,8 +236,11 @@ DataStoreService.prototype = { // This method can be called in the child so we need to send a request // to the parent and create DataStore object here. new DataStoreServiceChild(aWindow, aName, function(aStores) { - debug("DataStoreServiceChild callback!"); + debug("DataStoreServiceChild success callback!"); self.getDataStoreCreate(aWindow, resolve, aStores); + }, function() { + debug("DataStoreServiceChild error callback!"); + reject(new aWindow.DOMError("SecurityError", "Access denied")); }); } }); @@ -425,32 +428,38 @@ DataStoreService.prototype = { /* DataStoreServiceChild */ -function DataStoreServiceChild(aWindow, aName, aCallback) { +function DataStoreServiceChild(aWindow, aName, aSuccessCb, aErrorCb) { debug("DataStoreServiceChild created"); - this.init(aWindow, aName, aCallback); + this.init(aWindow, aName, aSuccessCb, aErrorCb); } DataStoreServiceChild.prototype = { __proto__: DOMRequestIpcHelper.prototype, - init: function(aWindow, aName, aCallback) { + init: function(aWindow, aName, aSuccessCb, aErrorCb) { debug("DataStoreServiceChild init"); - this._callback = aCallback; + this._successCb = aSuccessCb; + this._errorCb = aErrorCb; - this.initDOMRequestHelper(aWindow, [ "DataStore:Get:Return" ]); + this.initDOMRequestHelper(aWindow, [ "DataStore:Get:Return:OK", + "DataStore:Get:Return:KO" ]); - // This is a security issue and it will be fixed by Bug 916091 cpmm.sendAsyncMessage("DataStore:Get", - { name: aName, appId: aWindow.document.nodePrincipal.appId }); + { name: aName }, null, aWindow.document.nodePrincipal ); }, receiveMessage: function(aMessage) { debug("DataStoreServiceChild receiveMessage"); - if (aMessage.name != 'DataStore:Get:Return') { - return; - } - this._callback(aMessage.data.stores); + switch (aMessage.name) { + case 'DataStore:Get:Return:OK': + this._successCb(aMessage.data.stores); + break; + + case 'DataStore:Get:Return:KO': + this._errorCb(); + break; + } } } diff --git a/dom/datastore/DataStoreServiceInternal.jsm b/dom/datastore/DataStoreServiceInternal.jsm index 8bafc84c70b..3bbbeee3958 100644 --- a/dom/datastore/DataStoreServiceInternal.jsm +++ b/dom/datastore/DataStoreServiceInternal.jsm @@ -42,9 +42,14 @@ this.DataStoreServiceInternal = { let msg = aMessage.data; - // This is a security issue and it will be fixed by Bug 916091 - msg.stores = dataStoreService.getDataStoresInfo(msg.name, msg.appId); - aMessage.target.sendAsyncMessage("DataStore:Get:Return", msg); + if (!aMessage.principal || + aMessage.principal.appId == Ci.nsIScriptSecurityManager.UNKNOWN_APP_ID) { + aMessage.target.sendAsyncMessage("DataStore:Get:Return:KO"); + return; + } + + msg.stores = dataStoreService.getDataStoresInfo(msg.name, aMessage.principal.appId); + aMessage.target.sendAsyncMessage("DataStore:Get:Return:OK", msg); } } diff --git a/dom/datastore/tests/test_app_install.html b/dom/datastore/tests/test_app_install.html index a51b78854ca..9ff3c1466cc 100644 --- a/dom/datastore/tests/test_app_install.html +++ b/dom/datastore/tests/test_app_install.html @@ -26,7 +26,8 @@ { "type": "webapps-manage", "allow": 1, "context": document }], function() { SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true], - ["dom.promise.enabled", true]]}, function() { + ["dom.promise.enabled", true], + ["geo.testing.ignore_ipc_principal", true]]}, function() { gGenerator.next(); }); }); diff --git a/dom/datastore/tests/test_arrays.html b/dom/datastore/tests/test_arrays.html index 1d2212d59ad..0ed14cc393a 100644 --- a/dom/datastore/tests/test_arrays.html +++ b/dom/datastore/tests/test_arrays.html @@ -83,6 +83,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + function() { SpecialPowers.setAllAppsLaunchable(true); SpecialPowers.setBoolPref("dom.mozBrowserFramesEnabled", true); diff --git a/dom/datastore/tests/test_basic.html b/dom/datastore/tests/test_basic.html index c7ab9f17f38..12b3912b6eb 100644 --- a/dom/datastore/tests/test_basic.html +++ b/dom/datastore/tests/test_basic.html @@ -83,6 +83,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + function() { SpecialPowers.setAllAppsLaunchable(true); SpecialPowers.setBoolPref("dom.mozBrowserFramesEnabled", true); diff --git a/dom/datastore/tests/test_bug924104.html b/dom/datastore/tests/test_bug924104.html index e8d948e3b89..45b673a8f3e 100644 --- a/dom/datastore/tests/test_bug924104.html +++ b/dom/datastore/tests/test_bug924104.html @@ -83,6 +83,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + function() { SpecialPowers.setAllAppsLaunchable(true); SpecialPowers.setBoolPref("dom.mozBrowserFramesEnabled", true); diff --git a/dom/datastore/tests/test_changes.html b/dom/datastore/tests/test_changes.html index 138c3499cc2..ddc5c5f571f 100644 --- a/dom/datastore/tests/test_changes.html +++ b/dom/datastore/tests/test_changes.html @@ -127,6 +127,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + // Enabling mozBrowser function() { SpecialPowers.setAllAppsLaunchable(true); diff --git a/dom/datastore/tests/test_oop.html b/dom/datastore/tests/test_oop.html index f09cdf64a3f..f8b22db14ff 100644 --- a/dom/datastore/tests/test_oop.html +++ b/dom/datastore/tests/test_oop.html @@ -83,6 +83,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + function() { SpecialPowers.pushPrefEnv({"set": [["dom.ipc.browser_frames.oop_by_default", true]]}, runTest); }, diff --git a/dom/datastore/tests/test_readonly.html b/dom/datastore/tests/test_readonly.html index ce524de571e..73e8b1336f4 100644 --- a/dom/datastore/tests/test_readonly.html +++ b/dom/datastore/tests/test_readonly.html @@ -101,7 +101,8 @@ SimpleTest.waitForExplicitFinish(); SpecialPowers.pushPrefEnv({"set": [["dom.promise.enabled", true], - ["dom.datastore.enabled", true]]}, runTest); + ["dom.datastore.enabled", true], + ["geo.testing.ignore_ipc_principal", true]]}, runTest); diff --git a/dom/datastore/tests/test_sync.html b/dom/datastore/tests/test_sync.html index 2d20e040ee9..3a591300896 100644 --- a/dom/datastore/tests/test_sync.html +++ b/dom/datastore/tests/test_sync.html @@ -83,6 +83,10 @@ SpecialPowers.pushPrefEnv({"set": [["dom.datastore.enabled", true]]}, runTest); }, + function() { + SpecialPowers.pushPrefEnv({"set": [["geo.testing.ignore_ipc_principal", true]]}, runTest); + }, + function() { SpecialPowers.setBoolPref("dom.mozBrowserFramesEnabled", true); runTest(); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index d9cace523fa..935ca0a979b 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -2711,7 +2711,8 @@ ContentParent::RecvSyncMessage(const nsString& aMsg, InfallibleTArray* aRetvals) { nsIPrincipal* principal = aPrincipal; - if (principal && !AssertAppPrincipal(this, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(this, principal)) { return false; } @@ -2734,7 +2735,8 @@ ContentParent::AnswerRpcMessage(const nsString& aMsg, InfallibleTArray* aRetvals) { nsIPrincipal* principal = aPrincipal; - if (principal && !AssertAppPrincipal(this, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(this, principal)) { return false; } @@ -2755,7 +2757,8 @@ ContentParent::RecvAsyncMessage(const nsString& aMsg, const IPC::Principal& aPrincipal) { nsIPrincipal* principal = aPrincipal; - if (principal && !AssertAppPrincipal(this, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(this, principal)) { return false; } diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 80c6cbeab8b..7a2ffab196e 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -775,7 +775,8 @@ TabParent::RecvSyncMessage(const nsString& aMessage, { nsIPrincipal* principal = aPrincipal; ContentParent* parent = static_cast(Manager()); - if (principal && !AssertAppPrincipal(parent, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(parent, principal)) { return false; } @@ -793,7 +794,8 @@ TabParent::AnswerRpcMessage(const nsString& aMessage, { nsIPrincipal* principal = aPrincipal; ContentParent* parent = static_cast(Manager()); - if (principal && !AssertAppPrincipal(parent, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(parent, principal)) { return false; } @@ -810,7 +812,8 @@ TabParent::RecvAsyncMessage(const nsString& aMessage, { nsIPrincipal* principal = aPrincipal; ContentParent* parent = static_cast(Manager()); - if (principal && !AssertAppPrincipal(parent, principal)) { + if (!Preferences::GetBool("geo.testing.ignore_ipc_principal", false) && + principal && !AssertAppPrincipal(parent, principal)) { return false; }