Bug 1162281 - Invalid system message handler in an App Manifest can break the entire system. r=fabrice

dom/apps/Webapps.jsm

@@ -860,6 +860,10 @@ this.DOMApplicationRegistry = {
 
     if (!root.messages || !Array.isArray(root.messages) ||
         root.messages.length == 0) {
+      dump("Could not register invalid system message entry\n");
+      try {
+        dump(JSON.stringify(root.messages) + "\n");
+      } catch(e) {}
       return;
     }
 
@@ -869,28 +873,32 @@ this.DOMApplicationRegistry = {
     root.messages.forEach(function registerPages(aMessage) {
       let handlerPageURI = launchPathURI;
       let messageName;
-      if (typeof(aMessage) === "object" && Object.keys(aMessage).length === 1) {
-        messageName = Object.keys(aMessage)[0];
-        let handlerPath = aMessage[messageName];
-        // Resolve the handler path from origin. If |handler_path| is absent,
-        // simply skip.
-        let fullHandlerPath;
+      if (typeof(aMessage) !== "object" || Object.keys(aMessage).length !== 1) {
+        dump("Could not register invalid system message entry\n");
         try {
-          if (handlerPath && handlerPath.trim()) {
-            fullHandlerPath = manifest.resolveURL(handlerPath);
-          } else {
-            throw new Error("Empty or blank handler path.");
-          }
-        } catch(e) {
-          debug("system message handler path (" + handlerPath + ") is " +
-                "invalid, skipping. Error is: " + e);
-          return;
-        }
-        handlerPageURI = Services.io.newURI(fullHandlerPath, null, null);
-      } else {
-        messageName = aMessage;
+          dump(JSON.stringify(aMessage) + "\n");
+        } catch(e) {}
+        return;
       }
 
+      messageName = Object.keys(aMessage)[0];
+      let handlerPath = aMessage[messageName];
+      // Resolve the handler path from origin. If |handler_path| is absent,
+      // simply skip.
+      let fullHandlerPath;
+      try {
+        if (handlerPath && handlerPath.trim()) {
+          fullHandlerPath = manifest.resolveURL(handlerPath);
+        } else {
+          throw new Error("Empty or blank handler path.");
+        }
+      } catch(e) {
+        debug("system message handler path (" + handlerPath + ") is " +
+              "invalid, skipping. Error is: " + e);
+        return;
+      }
+      handlerPageURI = Services.io.newURI(fullHandlerPath, null, null);
+
       if (SystemMessagePermissionsChecker
             .isSystemMessagePermittedToRegister(messageName,
                                                 aApp.manifestURL,

dom/messages/SystemMessageInternal.js

@@ -273,7 +273,7 @@ SystemMessageInternal.prototype = {
                                    type: aType,
                                    msg: aMessage,
                                    extra: aExtra });
-      return;
+      return Promise.resolve();
     }
 
     // Give this message an ID so that we can identify the message and
@@ -285,37 +285,50 @@ SystemMessageInternal.prototype = {
 
     let shouldDispatchFunc = this._getMessageConfigurator(aType).shouldDispatch;
 
-    // Find pages that registered an handler for this type.
-    this._pages.forEach(function(aPage) {
-      if (aPage.type !== aType) {
-        return;
-      }
+    if (!this._pages.length) {
+      return Promise.resolve();
+    }
 
-      let doDispatch = () => {
-        let result = this._sendMessageCommon(aType,
-                                             aMessage,
-                                             messageID,
-                                             aPage.pageURL,
-                                             aPage.manifestURL,
-                                             aExtra);
-        debug("Returned status of sending message: " + result);
-      };
-
-      if ('function' !== typeof shouldDispatchFunc) {
-        // If the configurator has no 'shouldDispatch' defined,
-        // always dispatch this message.
-        doDispatch();
-        return;
-      }
-
-      shouldDispatchFunc(aPage.manifestURL, aPage.pageURL, aType, aMessage, aExtra)
-        .then(aShouldDispatch => {
-          if (aShouldDispatch) {
-            doDispatch();
+    // Find pages that registered a handler for this type.
+    let promises = [];
+    for (let i = 0; i < this._pages.length; i++) {
+      let promise = ((page) => {
+        return new Promise((resolve, reject) => {
+          if (page.type !== aType) {
+            resolve();
+            return;
           }
-        });
 
-    }, this);
+          let doDispatch = () => {
+            let result = this._sendMessageCommon(aType,
+                                                 aMessage,
+                                                 messageID,
+                                                 page.pageURL,
+                                                 page.manifestURL,
+                                                 aExtra);
+            debug("Returned status of sending message: " + result);
+            resolve();
+          };
+
+          if ('function' !== typeof shouldDispatchFunc) {
+            // If the configurator has no 'shouldDispatch' defined,
+            // always dispatch this message.
+            doDispatch();
+            return;
+          }
+
+          shouldDispatchFunc(page.manifestURL, page.pageURL, aType, aMessage, aExtra)
+            .then(aShouldDispatch => {
+              if (aShouldDispatch) {
+                doDispatch();
+              }
+            });
+        });
+      })(this._pages[i]);
+      promises.push(promise);
+    }
+
+    return Promise.all(promises);
   },
 
   registerPage: function(aType, aPageURI, aManifestURI) {

dom/messages/interfaces/nsISystemMessagesInternal.idl

@@ -10,7 +10,7 @@ interface nsIMessageSender;
 
 // Implemented by the contract id @mozilla.org/system-message-internal;1
 
-[scriptable, uuid(54c8e274-decb-4258-9a24-4ebfcbf3d00a)]
+[scriptable, uuid(59b6beda-f911-4d47-a296-8c81e6abcfb9)]
 interface nsISystemMessagesInternal : nsISupports
 {
   /*
@@ -36,8 +36,10 @@ interface nsISystemMessagesInternal : nsISupports
    * @param message     The message payload.
    * @param extra       Extra opaque information that will be passed around in the observer
    *                    notification to open the page.
+   * returns a Promise
    */
-  void broadcastMessage(in DOMString type, in jsval message, [optional] in jsval extra);
+  nsISupports broadcastMessage(in DOMString type, in jsval message,
+                               [optional] in jsval extra);
 
   /*
    * Registration of a page that wants to be notified of a message type.

dom/messages/test/chrome.ini

@@ -1,3 +1,11 @@
-[test_hasPendingMessage.html]
+[DEFAULT]
 skip-if = (buildapp != "browser") || e10s
-support-files = file_hasPendingMessage.html
+support-files =
+  file_hasPendingMessage.html
+  invalid_manifest.webapp
+  invalid_manifest.webapp^headers^
+  manifest.webapp
+  manifest.webapp^headers^
+
+[test_hasPendingMessage.html]
+[test_sysmsg_registration.html]

dom/messages/test/invalid_manifest.webapp

@@ -0,0 +1,8 @@
+{
+  "name": "Random app",
+  "launch_path": "/index.html",
+  "messages": [{
+    "dummy-system-message": "/index.html",
+    "dummy-system-message2": "/index.html"
+  }]
+}

dom/messages/test/invalid_manifest.webapp^headers^

@@ -0,0 +1 @@
+Content-Type: application/manifest+json

dom/messages/test/manifest.webapp

@@ -0,0 +1,9 @@
+{
+  "name": "Random app",
+  "launch_path": "/index.html",
+  "messages": [{
+    "dummy-system-message": "/index.html"
+  }, {
+    "dummy-system-message2": "/index.html"
+  }]
+}

dom/messages/test/manifest.webapp^headers^

@@ -0,0 +1 @@
+Content-Type: application/manifest+json

dom/messages/test/test_sysmsg_registration.html

@@ -0,0 +1,235 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>System messages registration tests</title>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/chrome-harness.js"></script>
+  <link rel="stylesheet" type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1162281}">Mozilla Bug {1162281}</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="application/javascript;version=1.7">
+
+const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+
+XPCOMUtils.defineLazyGetter(this, "messenger", () => {
+   return Cc["@mozilla.org/system-message-internal;1"]
+   .getService(Ci.nsISystemMessagesInternal);
+});
+
+let gRootUrl = "http://test/chrome/dom/messages/test/";
+let validAppUrl = gRootUrl + "manifest.webapp";
+let invalidAppUrl = gRootUrl + "invalid_manifest.webapp";
+let validApp;
+let initialAppsCount;
+let index = 0;
+
+SimpleTest.waitForExplicitFinish();
+
+function go() {
+  SpecialPowers.pushPermissions(
+    [{ "type": "webapps-manage", "allow": 1, "context": document },
+     { "type": "browser", "allow": 1, "context": document },
+     { "type": "embed-apps", "allow": 1, "context": document }],
+    function() {
+      SpecialPowers.pushPrefEnv(
+        {'set': [["dom.mozBrowserFramesEnabled", true],
+                 ["dom.sysmsg.enabled", true]]},
+        next) });
+}
+
+function finish() {
+  unregisterComponent(SystemMessageGlue);
+  SimpleTest.finish();
+}
+
+function next() {
+  info("Step " + index);
+  if (index >= steps.length) {
+    ok(false, "Shouldn't get here!");
+    return;
+  }
+  try {
+    let i = index++;
+    steps[i]();
+  } catch(ex) {
+    ok(false, "Caught exception", ex);
+  }
+}
+
+function cbError(aEvent) {
+  ok(false, "Error callback invoked " +
+            aEvent.target.error.name + " " + aEvent.target.error.message);
+  finish();
+}
+
+function uninstall(aApp) {
+  info("Uninstalling " + (aApp ? aApp.manifestURL : "NO APP!!"));
+}
+
+function registerComponent(aObject, aDescription, aContract) {
+  let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"]
+                        .getService(Ci.nsIUUIDGenerator);
+  let cid = uuidGenerator.generateUUID();
+
+  info("Registering " + cid);
+
+  let componentManager =
+    Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
+  componentManager.registerFactory(cid, aDescription, aContract, aObject);
+
+  // Keep the id on the object so we can unregister later.
+  aObject.cid = cid;
+}
+
+function unregisterComponent(aObject) {
+  info("Unregistering " + aObject.cid);
+  let componentManager =
+    Components.manager.QueryInterface(Ci.nsIComponentRegistrar);
+  componentManager.unregisterFactory(aObject.cid, aObject);
+}
+
+let SystemMessageGlue = {
+  // nsISupports implementation.
+  QueryInterface: function(iid) {
+    if (iid.equals(Ci.nsISupports) ||
+        iid.equals(Ci.nsIFactory) ||
+        iid.equals(Ci.nsISystemMessageGlue)) {
+      return this;
+    }
+
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  },
+
+  // nsIFactory implementation.
+  createInstance: function(outer, iid) {
+    return this.QueryInterface(iid);
+  },
+
+  _lastManifestURL: null,
+
+  // nsISystemMessageGlue implementation.
+  openApp(pageURL, manifestURL, type, target, showApp, onlyShowApp, extra) {
+    this._lastManifestURL = manifestURL;
+  }
+};
+
+registerComponent(SystemMessageGlue,
+                  "System Message Glue",
+                  "@mozilla.org/dom/messages/system-message-glue;1");
+
+function testBroadcastMessage(aMessage, aExpectedManifestURL, aMsg) {
+  SystemMessageGlue._lastManifestURL = null;
+  messenger.broadcastMessage(aMessage, {}, null)
+  .then(() => {
+    is(SystemMessageGlue._lastManifestURL, aExpectedManifestURL, aMsg);
+    next();
+  })
+  .catch(cbError);
+}
+
+/**
+ * TESTS
+ */
+let steps = [() => {
+  Services.obs.notifyObservers(null, "webapps-registry-ready", null);
+  SpecialPowers.setAllAppsLaunchable(true);
+  SpecialPowers.autoConfirmAppInstall(next);
+}, () => {
+  SpecialPowers.autoConfirmAppUninstall(next);
+}, () => {
+  // Check how many apps we are starting with.
+  let request = navigator.mozApps.mgmt.getAll();
+  request.onerror = cbError;
+  request.onsuccess = () => {
+    initialAppsCount = request.result.length;
+    info("Starting with " + initialAppsCount + " apps installed.");
+    next();
+  };
+}, () => {
+  // We still have not installed any app handling dummy-system-message.
+  testBroadcastMessage("dummy-system-message", null,
+                       "no system message should be sent");
+}, () => {
+  testBroadcastMessage("dummy-system-message2", null,
+                       "no system message should be sent");
+}, () => {
+  navigator.mozApps.mgmt.oninstall = ()  => {
+    validApp = request.result;
+    next();
+  };
+  let request = navigator.mozApps.install(validAppUrl, { });
+  request.error = cbError;
+  request.onsuccess = () => {
+    validApp = request.result;
+  };
+}, () => {
+  // Installing the test app should register the system message.
+  testBroadcastMessage("dummy-system-message", validAppUrl,
+                       "system message should be sent");
+}, () => {
+  // Installing the test app should register the system message.
+  testBroadcastMessage("dummy-system-message2", validAppUrl,
+                       "system message should be sent");
+}, () => {
+  navigator.mozApps.mgmt.onuninstall = ()  => {
+    validApp = null;
+    next();
+  };
+  let request = navigator.mozApps.mgmt.uninstall(validApp);
+  request.onerror = cbError;
+}, () => {
+  // Uninstalling the app should unregister the system messages.
+  testBroadcastMessage("dummy-system-message", null,
+                       "no system message should be sent");
+}, () => {
+  // Uninstalling the app should unregister the system messages.
+  testBroadcastMessage("dummy-system-message2", null,
+                       "no system message should be sent");
+}, () => {
+  navigator.mozApps.mgmt.oninstall = ()  => {
+    validApp = request.result;
+    next();
+  };
+  let request = navigator.mozApps.install(invalidAppUrl, { });
+  request.onerror = () => {
+    ok(true, "should not install invalid app");
+    next();
+  };
+  request.onsuccess = () => {
+    ok(false, "should not install invalid app");
+    finish();
+  };
+}, () => {
+  testBroadcastMessage("dummy-system-message", null,
+                       "no system message should be sent");
+}, () => {
+  testBroadcastMessage("dummy-system-message2", null,
+                       "no system message should be sent");
+}, () => {
+   // Check how many apps we are finishing with.
+  let request = navigator.mozApps.mgmt.getAll();
+  request.onerror = cbError;
+  request.onsuccess = () => {
+    info("Finishing with " + request.result.length + " apps installed.");
+    is(initialAppsCount, request.result.length, "All apps are uninstalled");
+    next();
+  };
+}, finish];
+
+addLoadEvent(go);
+
+</script>
+</pre>
+</body>
+</html>