Bug 1322862 - Make __webDriverArguments et al. content safe; r=automatedtester
authorAndreas Tolfsen <ato@mozilla.com>
Sat, 10 Dec 2016 16:15:53 -1000
changeset 341030 ce216a8c272f759d04d15fbdd663889d05c0f901
parent 341029 c4441c1492fd729592539121f7400e565e53b75e
child 341031 18fcec7442456c0a5951e951b197d436bd8c35f4
push id86615
push userkwierso@gmail.com
push dateTue, 07 Feb 2017 01:52:08 +0000
treeherdermozilla-inbound@f0453084d86e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1322862
milestone54.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 1322862 - Make __webDriverArguments et al. content safe; r=automatedtester Properties created in a more privileged scope than content cause permission denied errors when they are accessed from a less privileged scope. This is the case when we assign a document unload handler from chrome to a sandbox with content privileges. A permission denied error will be thrown if this handler is inspected from the code evaluated inside the sandbox. By cloning the properties along with their functions and wrapping their reflectors, we can ensure they can be safely inspected from content. MozReview-Commit-ID: Hy5MYvdTsv8
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
@@ -108,17 +108,17 @@ evaluate.sandbox = function (sb, script,
     unloadHandler = () => reject(
         new JavaScriptError("Document was unloaded during execution"));
 
     // wrap in function
     if (!opts.directInject) {
       if (opts.async) {
         sb[CALLBACK] = sb[COMPLETE];
       }
-      sb[ARGUMENTS] = Cu.cloneInto(args, sb, {wrapReflectors: true});
+      sb[ARGUMENTS] = sandbox.cloneInto(args, sb);
 
       // callback function made private
       // so that introspection is possible
       // on the arguments object
       if (opts.async) {
         sb[CALLBACK] = sb[COMPLETE];
         src += `${ARGUMENTS}.push(rv => ${CALLBACK}(rv));`;
       }
@@ -133,30 +133,28 @@ evaluate.sandbox = function (sb, script,
     }
 
     // onerror is not hooked on by default because of the inability to
     // differentiate content errors from chrome errors.
     //
     // see bug 1128760 for more details
     if (opts.debug) {
       sb.window.onerror = (msg, url, line) => {
-        let err = new JavaScriptError(`${msg} at: ${url} line: ${line}`);
+        let err = new JavaScriptError(`${msg} at ${url}:${line}`);
         reject(err);
       };
     }
 
     // timeout and unload handlers
-    scriptTimeoutID = setTimeout(
-        timeoutHandler, opts.timeout || DEFAULT_TIMEOUT);
-    sb.window.addEventListener("unload", unloadHandler);
+    scriptTimeoutID = setTimeout(timeoutHandler, opts.timeout || DEFAULT_TIMEOUT);
+    sb.window.onunload = sandbox.cloneInto(unloadHandler, sb);
 
     let res;
     try {
-      res = Cu.evalInSandbox(
-          src, sb, "1.8", opts.filename || "dummy file", 0);
+      res = Cu.evalInSandbox(src, sb, "1.8", opts.filename || "dummy file", 0);
     } catch (e) {
       let err = new JavaScriptError(
           e,
           "execute_script",
           opts.filename,
           opts.line,
           script);
       reject(err);
@@ -172,16 +170,28 @@ evaluate.sandbox = function (sb, script,
     sb.window.removeEventListener("unload", unloadHandler);
     return res;
   });
 };
 
 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
+ * and DOM elemnets.
+ */
+sandbox.cloneInto = function (obj, sb) {
+  return Cu.cloneInto(obj, sb, {cloneFunctions: true, wrapReflectors: true});
+};
+
+/**
  * Augment given sandbox by an adapter that has an {@code exports}
  * map property, or a normal map, of function names and function
  * references.
  *
  * @param {Sandbox} sb
  *     The sandbox to augment.
  * @param {Object} adapter
  *     Object that holds an {@code exports} property, or a map, of
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py
@@ -257,16 +257,27 @@ class TestExecuteContent(MarionetteTestC
             "return window.n", sandbox=None),
             "setTimeout already fired")
 
         # if event was cancelled, this will time out
         Wait(self.marionette, timeout=8).until(
             content_timeout_triggered,
             message="Scheduled setTimeout event was cancelled by call to execute_script")
 
+    def test_privileged_code_inspection(self):
+        # test permission denied on toString of unload event handler
+        self.marionette.navigate(inline("""
+            <script>
+            window.addEventListener = (type, handler) => handler.toString();
+            </script>"""))
+        self.marionette.execute_script("", sandbox=None)
+
+        # test inspection of arguments
+        self.marionette.execute_script("__webDriverArguments.toString()")
+
 
 class TestExecuteChrome(WindowManagerMixin, TestExecuteContent):
 
     def setUp(self):
         super(TestExecuteChrome, self).setUp()
 
         self.marionette.set_context("chrome")
 
@@ -332,16 +343,19 @@ class TestExecuteChrome(WindowManagerMix
         pass
 
     def test_return_web_element_nodelist(self):
         pass
 
     def test_window_set_timeout_is_not_cancelled(self):
         pass
 
+    def test_privileged_code_inspection(self):
+        pass
+
 
 class TestElementCollections(MarionetteTestCase):
 
     def assertSequenceIsInstance(self, seq, typ):
         for item in seq:
             self.assertIsInstance(item, typ)
 
     def test_array(self):