Bug 1368195 - Disallow XPCOM objects from being serialised by Marionette; r?whimboo, mccr8 draft
authorAndreas Tolfsen <ato@mozilla.com>
Sat, 27 May 2017 18:22:24 +0100
changeset 585652 24a1fd018ee06b8496cdb3cd929e5359d123c92c
parent 585651 690efe04cfeabf8c64f6efdebb7453fe91df43ae
child 630768 bfe526c27f51b780695d7f02c3341bf2e9b97622
push id61161
push userbmo:ato@mozilla.com
push dateSat, 27 May 2017 17:25:52 +0000
reviewerswhimboo, mccr8
bugs1368195
milestone55.0a1
Bug 1368195 - Disallow XPCOM objects from being serialised by Marionette; r?whimboo, mccr8 This patch makes it impossible to return objects that implement XPCOM interfaces from Marionette's script evaluation module. When objects are instances of Components.interfaces.nsISupports and has a QueryInterface own property, a JavaScript error is returned with a helpful explanation why it is a bad idea. MozReview-Commit-ID: H9jfcgiLDWr
testing/marionette/evaluate.js
testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
--- a/testing/marionette/evaluate.js
+++ b/testing/marionette/evaluate.js
@@ -265,36 +265,43 @@ evaluate.toJSON = function (obj, seenEls
   }
 
   // HTMLElement
   else if ("nodeType" in obj && obj.nodeType == obj.ELEMENT_NODE) {
     let uuid = seenEls.add(obj);
     return element.makeWebElement(uuid);
   }
 
+  // XPCOM object
+  else if (evaluate.isXPCOMObject(obj)) {
+    throw new JavaScriptError("Unable to safely serialise XPCOM object: " + obj);
+  }
+
   // custom JSON representation
   else if (typeof obj["toJSON"] == "function") {
     let unsafeJSON = obj.toJSON();
     return evaluate.toJSON(unsafeJSON, seenEls);
   }
 
   // arbitrary objects + files
   else {
     let rv = {};
     for (let prop in obj) {
-      try {
-        rv[prop] = evaluate.toJSON(obj[prop], seenEls);
-      } catch (e if (e.result == Cr.NS_ERROR_NOT_IMPLEMENTED)) {
-        logger.debug(`Skipping ${prop}: ${e.message}`);
-      }
+      rv[prop] = evaluate.toJSON(obj[prop], seenEls);
     }
     return rv;
   }
 };
 
+/** Returns true if |obj| is an XPCOM object instance. */
+evaluate.isXPCOMObject = function (obj) {
+  return obj instanceof Ci.nsISupports &&
+      "QueryInterface" in obj;
+};
+
 this.sandbox = {};
 
 /**
  * Provides a safe way to take an object defined in a privileged scope and
  * create a structured clone of it in a less-privileged scope.  It returns
  * a reference to the clone.
  *
  * Unlike for |Components.utils.cloneInto|, |obj| may contain functions
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
@@ -348,16 +348,20 @@ class TestExecuteContent(MarionetteTestC
             return {
               toJSON () {
                 return document.documentElement;
               }
             }""",
             sandbox=None)
         self.assert_is_web_element(el)
 
+    def test_xpcom_object(self):
+        with self.assertRaises(errors.JavascriptException):
+            self.marionette.execute_script("return Services.appinfo")
+
 
 class TestExecuteChrome(WindowManagerMixin, TestExecuteContent):
 
     def setUp(self):
         super(TestExecuteChrome, self).setUp()
 
         self.marionette.set_context("chrome")