Bug 1403536 - Protect all protocol request methods against unsafe objects r=ochameau

MozReview-Commit-ID: 4fDaap9QCdF

--HG--
extra : rebase_source : 0774e230ddc9b8fe1bc03be45cd5732d3a161e41

devtools/client/webconsole/new-console-output/test/mochitest/browser_console.js

@@ -204,9 +204,43 @@ function* testCPOWInspection(hud) {
   // But it's only a CPOW in e10s.
   let e10sCheck = yield hud.jsterm.requestEvaluation(
     "Cu.isCrossProcessWrapper(gBrowser.selectedBrowser._contentWindow)");
-  if (e10sCheck.result) {
-    is(cpow.class, "CPOW: Window", "The CPOW grip has the right class.");
-  } else {
+  if (!e10sCheck.result) {
     is(cpow.class, "Window", "The object is not a CPOW.");
+    return;
   }
+
+  is(cpow.class, "CPOW: Window", "The CPOW grip has the right class.");
+
+  // Check that various protocol request methods work for the CPOW.
+  let response, slice;
+  let objClient = new ObjectClient(hud.jsterm.hud.proxy.client, cpow);
+
+  response = yield objClient.getPrototypeAndProperties();
+  is(Reflect.ownKeys(response.ownProperties).length, 0, "No property was retrieved.");
+  is(response.ownSymbols.length, 0, "No symbol property was retrieved.");
+  is(response.prototype.type, "null", "The prototype is null.");
+
+  response = yield objClient.enumProperties({ignoreIndexedProperties: true});
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(Reflect.ownKeys(slice.ownProperties).length, 0, "No property was retrieved.");
+
+  response = yield objClient.enumProperties({});
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(Reflect.ownKeys(slice.ownProperties).length, 0, "No property was retrieved.");
+
+  response = yield objClient.getOwnPropertyNames();
+  is(response.ownPropertyNames.length, 0, "No property was retrieved.");
+
+  response = yield objClient.getProperty("x");
+  is(response.descriptor, undefined, "The property does not exist.");
+
+  response = yield objClient.enumSymbols();
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(slice.ownSymbols.length, 0, "No symbol property was retrieved.");
+
+  response = yield objClient.getPrototype();
+  is(response.prototype.type, "null", "The prototype is null.");
+
+  response = yield objClient.getDisplayString();
+  is(response.displayString, "<cpow>", "The CPOW stringifies to <cpow>");
 }

devtools/client/webconsole/test/browser_console.js

@@ -204,9 +204,43 @@ function* testCPOWInspection(hud) {
   // But it's only a CPOW in e10s.
   let e10sCheck = yield hud.jsterm.requestEvaluation(
     "Cu.isCrossProcessWrapper(gBrowser.selectedBrowser._contentWindow)");
-  if (e10sCheck.result) {
-    is(cpow.class, "CPOW: Window", "The CPOW grip has the right class.");
-  } else {
+  if (!e10sCheck.result) {
     is(cpow.class, "Window", "The object is not a CPOW.");
+    return;
   }
+
+  is(cpow.class, "CPOW: Window", "The CPOW grip has the right class.");
+
+  // Check that various protocol request methods work for the CPOW.
+  let response, slice;
+  let objClient = new ObjectClient(hud.jsterm.hud.proxy.client, cpow);
+
+  response = yield objClient.getPrototypeAndProperties();
+  is(Reflect.ownKeys(response.ownProperties).length, 0, "No property was retrieved.");
+  is(response.ownSymbols.length, 0, "No symbol property was retrieved.");
+  is(response.prototype.type, "null", "The prototype is null.");
+
+  response = yield objClient.enumProperties({ignoreIndexedProperties: true});
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(Reflect.ownKeys(slice.ownProperties).length, 0, "No property was retrieved.");
+
+  response = yield objClient.enumProperties({});
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(Reflect.ownKeys(slice.ownProperties).length, 0, "No property was retrieved.");
+
+  response = yield objClient.getOwnPropertyNames();
+  is(response.ownPropertyNames.length, 0, "No property was retrieved.");
+
+  response = yield objClient.getProperty("x");
+  is(response.descriptor, undefined, "The property does not exist.");
+
+  response = yield objClient.enumSymbols();
+  slice = yield response.iterator.slice(0, response.iterator.count);
+  is(slice.ownSymbols.length, 0, "No symbol property was retrieved.");
+
+  response = yield objClient.getPrototype();
+  is(response.prototype.type, "null", "The prototype is null.");
+
+  response = yield objClient.getDisplayString();
+  is(response.displayString, "<cpow>", "The CPOW stringifies to <cpow>");
 }

devtools/server/actors/object.js

@@ -10,7 +10,7 @@ const { Cu, Ci } = require("chrome");
 const { GeneratedLocation } = require("devtools/server/actors/common");
 const { DebuggerServer } = require("devtools/server/main");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
-const { assert, dumpn } = DevToolsUtils;
+const { assert } = DevToolsUtils;
 
 loader.lazyRequireGetter(this, "ChromeUtils");
 
@@ -75,40 +75,44 @@ ObjectActor.prototype = {
   grip: function () {
     let g = {
       "type": "object",
-      "actor": this.actorID
+      "actor": this.actorID,
+      "class": this.obj.class,
     };
 
-    // Check if the object has a wrapper which denies access. It may be a CPOW or a
-    // security wrapper. Change the class so that this will be visible in the UI.
     let unwrapped = DevToolsUtils.unwrap(this.obj);
-    if (!unwrapped) {
+
+    // Unsafe objects must be treated carefully.
+    if (!DevToolsUtils.isSafeDebuggerObject(this.obj)) {
       if (DevToolsUtils.isCPOW(this.obj)) {
-        g.class = "CPOW: " + this.obj.class;
-      } else {
-        g.class = "Inaccessible";
+        // Cross-process object wrappers can't be accessed.
+        g.class = "CPOW: " + g.class;
+      } else if (unwrapped === undefined) {
+        // Objects belonging to an invisible-to-debugger compartment might be proxies,
+        // so just in case they shouldn't be accessed.
+        g.class = "InvisibleToDebugger: " + g.class;
+      } else if (unwrapped.isProxy) {
+        // Proxy objects can run traps when accessed, so just create a preview with
+        // the target and the handler.
+        g.class = "Proxy";
+        this.hooks.incrementGripDepth();
+        DebuggerServer.ObjectActorPreviewers.Proxy[0](this, g, null);
+        this.hooks.decrementGripDepth();
       }
       return g;
     }
 
-    // Dead objects also deny access.
-    if (this.obj.class == "DeadObject") {
-      g.class = "DeadObject";
-      return g;
+    // If the debuggee does not subsume the object's compartment, most properties won't
+    // be accessible. Cross-orgin Window and Location objects might expose some, though.
+    // Change the displayed class, but when creating the preview use the original one.
+    if (unwrapped === null) {
+      g.class = "Restricted";
     }
 
-    // Otherwise, increment grip depth and attempt to create a preview.
     this.hooks.incrementGripDepth();
 
-    // The `isProxy` getter is called on `unwrapped` instead of `this.obj` in order
-    // to detect proxies behind transparent wrappers, and thus avoid running traps.
-    if (unwrapped.isProxy) {
-      g.class = "Proxy";
-    } else {
-      g.class = this.obj.class;
-      g.extensible = this.obj.isExtensible();
-      g.frozen = this.obj.isFrozen();
-      g.sealed = this.obj.isSealed();
-    }
+    g.extensible = this.obj.isExtensible();
+    g.frozen = this.obj.isFrozen();
+    g.sealed = this.obj.isSealed();
 
     if (g.class == "Promise") {
       g.promiseState = this._createPromiseState();
@@ -119,8 +123,13 @@ ObjectActor.prototype = {
     if (isTypedArray(g)) {
       // Bug 1348761: getOwnPropertyNames is unnecessary slow on TypedArrays
       g.ownPropertyLength = getArrayLength(this.obj);
-    } else if (g.class != "Proxy") {
-      g.ownPropertyLength = this.obj.getOwnPropertyNames().length;
+    } else {
+      try {
+        g.ownPropertyLength = this.obj.getOwnPropertyNames().length;
+      } catch (err) {
+        // The above can throw when the debuggee does not subsume the object's
+        // compartment, or for some WrappedNatives like Cu.Sandbox.
+      }
     }
 
     let raw = this.obj.unsafeDereference();
@@ -135,7 +144,7 @@ ObjectActor.prototype = {
       raw = null;
     }
 
-    let previewers = DebuggerServer.ObjectActorPreviewers[g.class] ||
+    let previewers = DebuggerServer.ObjectActorPreviewers[this.obj.class] ||
                      DebuggerServer.ObjectActorPreviewers.Object;
     for (let fn of previewers) {
       try {
@@ -226,8 +235,16 @@ ObjectActor.prototype = {
    * the object and not its prototype.
    */
   onOwnPropertyNames: function () {
-    return { from: this.actorID,
-             ownPropertyNames: this.obj.getOwnPropertyNames() };
+    let props = [];
+    if (DevToolsUtils.isSafeDebuggerObject(this.obj)) {
+      try {
+        props = this.obj.getOwnPropertyNames();
+      } catch (err) {
+        // The above can throw when the debuggee does not subsume the object's
+        // compartment, or for some WrappedNatives like Cu.Sandbox.
+      }
+    }
+    return { from: this.actorID, ownPropertyNames: props };
   },
 
   /**
@@ -282,21 +299,22 @@ ObjectActor.prototype = {
    *                     with safe getters descriptors.
    */
   onPrototypeAndProperties: function () {
-    let ownProperties = Object.create(null);
-    let ownSymbols = [];
-
-    // Inaccessible, proxy and dead objects should not be accessed.
-    let unwrapped = DevToolsUtils.unwrap(this.obj);
-    if (!unwrapped || unwrapped.isProxy || this.obj.class == "DeadObject") {
-      return { from: this.actorID,
-               prototype: this.hooks.createValueGrip(null),
-               ownProperties,
-               ownSymbols,
-               safeGetterValues: Object.create(null) };
+    let proto = null;
+    let names = [];
+    let symbols = [];
+    if (DevToolsUtils.isSafeDebuggerObject(this.obj)) {
+      try {
+        proto = this.obj.proto;
+        names = this.obj.getOwnPropertyNames();
+        symbols = this.obj.getOwnPropertySymbols();
+      } catch (err) {
+        // The above can throw when the debuggee does not subsume the object's
+        // compartment, or for some WrappedNatives like Cu.Sandbox.
+      }
     }
 
-    let names = this.obj.getOwnPropertyNames();
-    let symbols = this.obj.getOwnPropertySymbols();
+    let ownProperties = Object.create(null);
+    let ownSymbols = [];
 
     for (let name of names) {
       ownProperties[name] = this._propertyDescriptor(name);
@@ -310,7 +328,7 @@ ObjectActor.prototype = {
     }
 
     return { from: this.actorID,
-             prototype: this.hooks.createValueGrip(this.obj.proto),
+             prototype: this.hooks.createValueGrip(proto),
              ownProperties,
              ownSymbols,
              safeGetterValues: this._findSafeGetterValues(names) };
@@ -334,9 +352,8 @@ ObjectActor.prototype = {
     let obj = this.obj;
     let level = 0, i = 0;
 
-    // Do not search safe getters in inaccessible nor proxy objects.
-    let unwrapped = DevToolsUtils.unwrap(obj);
-    if (!unwrapped || unwrapped.isProxy) {
+    // Do not search safe getters in unsafe objects.
+    if (!DevToolsUtils.isSafeDebuggerObject(obj)) {
       return safeGetterValues;
     }
 
@@ -349,13 +366,7 @@ ObjectActor.prototype = {
       level++;
     }
 
-    while (obj) {
-      // Stop iterating when an inaccessible or a proxy object is found.
-      unwrapped = DevToolsUtils.unwrap(obj);
-      if (!unwrapped || unwrapped.isProxy) {
-        break;
-      }
-
+    while (obj && DevToolsUtils.isSafeDebuggerObject(obj)) {
       let getters = this._findSafeGetters(obj);
       for (let name of getters) {
         // Avoid overwriting properties from prototypes closer to this.obj. Also
@@ -434,6 +445,12 @@ ObjectActor.prototype = {
     }
 
     let getters = new Set();
+
+    if (!DevToolsUtils.isSafeDebuggerObject(object)) {
+      object._safeGetters = getters;
+      return getters;
+    }
+
     let names = [];
     try {
       names = object.getOwnPropertyNames();
@@ -467,8 +484,12 @@ ObjectActor.prototype = {
    * Handle a protocol request to provide the prototype of the object.
    */
   onPrototype: function () {
+    let proto = null;
+    if (DevToolsUtils.isSafeDebuggerObject(this.obj)) {
+      proto = this.obj.proto;
+    }
     return { from: this.actorID,
-             prototype: this.hooks.createValueGrip(this.obj.proto) };
+             prototype: this.hooks.createValueGrip(proto) };
   },
 
   /**
@@ -512,6 +533,10 @@ ObjectActor.prototype = {
    *         property and onlyEnumerable=true.
    */
   _propertyDescriptor: function (name, onlyEnumerable) {
+    if (!DevToolsUtils.isSafeDebuggerObject(this.obj)) {
+      return undefined;
+    }
+
     let desc;
     try {
       desc = this.obj.getOwnPropertyDescriptor(name);
@@ -801,7 +826,13 @@ ObjectActor.prototype.requestTypes = {
  *          of the property value.
  */
 function PropertyIteratorActor(objectActor, options) {
-  if (options.enumEntries) {
+  if (!DevToolsUtils.isSafeDebuggerObject(objectActor.obj)) {
+    this.iterator = {
+      size: 0,
+      propertyName: index => undefined,
+      propertyDescription: index => undefined,
+    };
+  } else if (options.enumEntries) {
     let cls = objectActor.obj.class;
     if (cls == "Map") {
       this.iterator = enumMapEntries(objectActor);
@@ -1161,7 +1192,15 @@ function enumWeakSetEntries(objectActor) {
  *        The object actor.
  */
 function SymbolIteratorActor(objectActor) {
-  const symbols =  objectActor.obj.getOwnPropertySymbols();
+  let symbols = [];
+  if (DevToolsUtils.isSafeDebuggerObject(objectActor.obj)) {
+    try {
+      symbols = objectActor.obj.getOwnPropertySymbols();
+    } catch (err) {
+      // The above can throw when the debuggee does not subsume the object's
+      // compartment, or for some WrappedNatives like Cu.Sandbox.
+    }
+  }
 
   this.iterator = {
     size: symbols.length,
@@ -1258,9 +1297,8 @@ DebuggerServer.ObjectActorPreviewers = {
     try {
       userDisplayName = obj.getOwnPropertyDescriptor("displayName");
     } catch (e) {
-      // Calling getOwnPropertyDescriptor with displayName might throw
-      // with "permission denied" errors for some functions.
-      dumpn(e);
+      // The above can throw "permission denied" errors when the debuggee
+      // does not subsume the function's compartment.
     }
 
     if (userDisplayName && typeof userDisplayName.value == "string" &&
@@ -1940,7 +1978,14 @@ DebuggerServer.ObjectActorPreviewers.Object = [
     // - The array indices are consecutive.
     // - The value of "length", if present, is the number of array indices.
 
-    let keys = obj.getOwnPropertyNames();
+    let keys;
+    try {
+      keys = obj.getOwnPropertyNames();
+    } catch (err) {
+      // The above can throw when the debuggee does not subsume the object's
+      // compartment, or for some WrappedNatives like Cu.Sandbox.
+      return false;
+    }
     let {length} = keys;
     if (length === 0) {
       return false;
@@ -2039,7 +2084,16 @@ function isObject(value) {
  *         The stringifier for the class.
  */
 function createBuiltinStringifier(ctor) {
-  return obj => ctor.prototype.toString.call(obj.unsafeDereference());
+  return obj => {
+    try {
+      return ctor.prototype.toString.call(obj.unsafeDereference());
+    } catch (err) {
+      // The debuggee will see a "Function" class if the object is callable and
+      // its compartment is not subsumed. The above will throw if it's not really
+      // a function, e.g. if it's a callable proxy.
+      return "[object " + obj.class + "]";
+    }
+  };
 }
 
 /**
@@ -2078,9 +2132,20 @@ function errorStringify(obj) {
  *         The stringification for the object.
  */
 function stringify(obj) {
-  if (obj.class == "DeadObject") {
-    const error = new Error("Dead object encountered.");
-    DevToolsUtils.reportException("stringify", error);
+  if (!DevToolsUtils.isSafeDebuggerObject(obj)) {
+    if (DevToolsUtils.isCPOW(obj)) {
+      return "<cpow>";
+    }
+    let unwrapped = DevToolsUtils.unwrap(obj);
+    if (unwrapped === undefined) {
+      return "<invisibleToDebugger>";
+    } else if (unwrapped.isProxy) {
+      return "<proxy>";
+    }
+    // The following line should not be reached. It's there just in case somebody
+    // modifies isSafeDebuggerObject to return false for additional kinds of objects.
+    return "[object " + obj.class + "]";
+  } else if (obj.class == "DeadObject") {
     return "<dead object>";
   }
 
@@ -2123,25 +2188,21 @@ var stringifiers = {
 
     seen.add(obj);
 
-    const len = DevToolsUtils.getProperty(obj, "length");
+    const len = getArrayLength(obj);
     let string = "";
 
-    // The following check is only required because the debuggee could possibly
-    // be a Proxy and return any value. For normal objects, array.length is
-    // always a non-negative integer.
-    if (typeof len == "number" && len > 0) {
-      for (let i = 0; i < len; i++) {
-        const desc = obj.getOwnPropertyDescriptor(i);
-        if (desc) {
-          const { value } = desc;
-          if (value != null) {
-            string += isObject(value) ? stringify(value) : value;
-          }
+    // Array.length is always a non-negative safe integer.
+    for (let i = 0; i < len; i++) {
+      const desc = obj.getOwnPropertyDescriptor(i);
+      if (desc) {
+        const { value } = desc;
+        if (value != null) {
+          string += isObject(value) ? stringify(value) : value;
         }
+      }
 
-        if (i < len - 1) {
-          string += ",";
-        }
+      if (i < len - 1) {
+        string += ",";
       }
     }
 

devtools/server/tests/unit/test_objectgrips-12.js

@@ -122,7 +122,7 @@ function test_display_string() {
     },
     {
       input: "new Proxy({}, {})",
-      output: "[object Object]"
+      output: "<proxy>"
     },
     {
       input: "Promise.resolve(5)",

devtools/server/tests/unit/test_objectgrips-17.js

@@ -172,7 +172,16 @@ function check_proxy_grip(grip) {
     // The proxy has opaque security wrappers.
     strictEqual(grip.class, "Opaque", "The grip has an Opaque class.");
     strictEqual(grip.ownPropertyLength, 0, "The grip has no properties.");
-  } else if (gSubsumes && !gGlobalIsInvisible) {
+  } else if (!gSubsumes) {
+    // The proxy belongs to compartment not subsumed by the debuggee.
+    strictEqual(grip.class, "Restricted", "The grip has an Restricted class.");
+    ok(!("ownPropertyLength" in grip), "The grip doesn't know the number of properties.");
+  } else if (gGlobalIsInvisible) {
+    // The proxy belongs to an invisible-to-debugger compartment.
+    strictEqual(grip.class, "InvisibleToDebugger: Object",
+                "The grip has an InvisibleToDebugger class.");
+    ok(!("ownPropertyLength" in grip), "The grip doesn't know the number of properties.");
+  } else {
     // The proxy has non-opaque security wrappers.
     strictEqual(grip.class, "Proxy", "The grip has a Proxy class.");
     ok(!("proxyTarget" in grip), "There is no [[ProxyTarget]] grip.");
@@ -180,10 +189,6 @@ function check_proxy_grip(grip) {
     strictEqual(preview.ownPropertiesLength, 0, "The preview has no properties.");
     ok(!("<target>" in preview), "The preview has no <target> property.");
     ok(!("<handler>" in preview), "The preview has no <handler> property.");
-  } else {
-    // The debuggee is not allowed to remove the security wrappers.
-    strictEqual(grip.class, "Inaccessible", "The grip has an Inaccessible class.");
-    ok(!("ownPropertyLength" in grip), "The grip doesn't know the number of properties.");
   }
 }
 
@@ -208,7 +213,8 @@ function check_prototype(proto, isProxy, createdInDebuggee) {
   } else if (isProxy && gIsOpaque && gGlobalIsInvisible) {
     // The object is a proxy with opaque security wrappers in an invisible global.
     // The debuggee sees an inaccessible `Object.prototype` when retrieving the prototype.
-    strictEqual(proto.class, "Inaccessible", "The prototype has an Inaccessible class.");
+    strictEqual(proto.class, "InvisibleToDebugger: Object",
+                "The prototype has an InvisibleToDebugger class.");
   } else if (createdInDebuggee || !isProxy && gSubsumes && !gGlobalIsInvisible) {
     // The object inherits from a proxy and has no security wrappers or non-opaque ones.
     // The debuggee sees the proxy when retrieving the prototype.

devtools/server/tests/unit/test_objectgrips-21.js

@@ -0,0 +1,388 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+/* eslint-disable no-shadow, max-nested-callbacks */
+
+"use strict";
+
+var gDebuggee;
+var gThreadClient;
+
+function run_test() {
+  run_test_with_server(DebuggerServer, function () {
+    run_test_with_server(WorkerDebuggerServer, do_test_finished);
+  });
+  do_test_pending();
+}
+
+async function run_test_with_server(server, callback) {
+  initTestDebuggerServer(server);
+  let principals = [
+    ["test-grips-system-principal", systemPrincipal, systemPrincipalTests],
+    ["test-grips-null-principal", null, nullPrincipalTests],
+  ];
+  for (let [title, principal, tests] of principals) {
+    gDebuggee = Cu.Sandbox(principal);
+    gDebuggee.__name = title;
+    server.addTestGlobal(gDebuggee);
+    gDebuggee.eval(function stopMe(arg1, arg2) {
+      debugger;
+    }.toString());
+    let client = new DebuggerClient(server.connectPipe());
+    await client.connect();
+    const [,, threadClient] = await attachTestTabAndResume(client, title);
+    gThreadClient = threadClient;
+    await test_unsafe_grips(principal, tests);
+    await client.close();
+  }
+  callback();
+}
+
+// The following tests work like this:
+// - The specified code is evaluated in a system principal.
+//   `Cu`, `systemPrincipal` and `Services` are provided as global variables.
+// - The resulting object is debugged in a system or null principal debuggee,
+//   depending on in which list the test is placed.
+//   It is tested according to the specified test parameters.
+// - An ordinary object that inherits from the resulting one is also debugged.
+//   This is just to check that it can be normally debugged even with an unsafe
+//   object in the prototype. The specified test parameters do not apply.
+
+// The following tests are defined via properties with the following defaults.
+let defaults = {
+  // The class of the grip.
+  class: "Restricted",
+
+  // The stringification of the object
+  string: "",
+
+  // Whether the object (not its grip) has class "Function".
+  isFunction: false,
+
+  // Whether the grip has a preview property.
+  hasPreview: true,
+
+  // Code that assigns the object to be tested into the obj variable.
+  code: "var obj = {}",
+
+  // The type of the grip of the prototype.
+  protoType: "null",
+
+  // Whether the object has some own string properties.
+  hasOwnPropertyNames: false,
+
+  // Whether the object has some own symbol properties.
+  hasOwnPropertySymbols: false,
+
+  // The descriptor obtained when retrieving property "x" or Symbol("x").
+  property: undefined,
+
+  // Code evaluated after the test, whose result is expected to be true.
+  afterTest: "true == true",
+};
+
+// Obtaining a CPOW here does not seem possible, so the CPOW test is in
+// devtools/client/webconsole/test/browser_console.js
+
+// The following tests use a system principal debuggee.
+let systemPrincipalTests = [{
+  // Dead objects throw a TypeError when accessing properties.
+  class: "DeadObject",
+  string: "<dead object>",
+  code: `
+    var obj = Cu.Sandbox(null);
+    Cu.nukeSandbox(obj);
+  `,
+  property: descriptor({value: "TypeError"}),
+}, {
+  // This proxy checks that no trap runs (using a second proxy as the handler
+  // there is no need to maintain a list of all possible traps).
+  class: "Proxy",
+  string: "<proxy>",
+  code: `
+    var trapDidRun = false;
+    var obj = new Proxy({}, new Proxy({}, {get: (_, trap) => {
+      trapDidRun = true;
+      throw new Error("proxy trap '" + trap + "' was called.");
+    }}));
+  `,
+  afterTest: "trapDidRun === false",
+}, {
+  // Like the previous test, but now the proxy has a Function class.
+  class: "Proxy",
+  string: "<proxy>",
+  isFunction: true,
+  code: `
+    var trapDidRun = false;
+    var obj = new Proxy(function(){}, new Proxy({}, {get: (_, trap) => {
+      trapDidRun = true;
+      throw new Error("proxy trap '" + trap + "' was called.");
+    }}));
+  `,
+  afterTest: "trapDidRun === false",
+}, {
+  // Invisisible-to-debugger objects can't be unwrapped, so we don't know if
+  // they are proxies. Thus they shouldn't be accessed.
+  class: "InvisibleToDebugger: Array",
+  string: "<invisibleToDebugger>",
+  hasPreview: false,
+  code: `
+    var s = Cu.Sandbox(systemPrincipal, {invisibleToDebugger: true});
+    var obj = s.eval("[1, 2, 3]");
+  `,
+}, {
+  // Like the previous test, but now the object has a Function class.
+  class: "InvisibleToDebugger: Function",
+  string: "<invisibleToDebugger>",
+  isFunction: true,
+  hasPreview: false,
+  code: `
+    var s = Cu.Sandbox(systemPrincipal, {invisibleToDebugger: true});
+    var obj = s.eval("(function func(arg){})");
+  `,
+}, {
+  // Cu.Sandbox is a WrappedNative that throws when accessing properties.
+  class: "nsXPCComponents_utils_Sandbox",
+  string: "[object nsXPCComponents_utils_Sandbox]",
+  code: `var obj = Cu.Sandbox;`,
+  protoType: "object",
+}];
+
+// The following tests run code in a system principal, but the resulting object
+// is debugged in a null principal.
+let nullPrincipalTests = [{
+  // The null principal gets undefined when attempting to access properties.
+  string: "[object Object]",
+  code: `var obj = {x: -1};`,
+}, {
+  // For arrays it's an error instead of undefined.
+  string: "[object Object]",
+  code: `var obj = [1, 2, 3];`,
+  property: descriptor({value: "Error"}),
+}, {
+  // For functions it's also an error.
+  string: "function func(arg){}",
+  isFunction: true,
+  hasPreview: false,
+  code: `var obj = function func(arg){};`,
+  property: descriptor({value: "Error"}),
+}, {
+  // Check that no proxy trap runs.
+  string: "[object Object]",
+  code: `
+    var trapDidRun = false;
+    var obj = new Proxy([], new Proxy({}, {get: (_, trap) => {
+      trapDidRun = true;
+      throw new Error("proxy trap '" + trap + "' was called.");
+    }}));
+  `,
+  property: descriptor({value: "Error"}),
+  afterTest: `trapDidRun === false`,
+}, {
+  // Like the previous test, but now the object has a Function class.
+  string: "[object Function]",
+  isFunction: true,
+  hasPreview: false,
+  code: `
+    var trapDidRun = false;
+    var obj = new Proxy(function(){}, new Proxy({}, {get: (_, trap) => {
+      trapDidRun = true;
+      throw new Error("proxy trap '" + trap + "' was called.");
+    }}));
+  `,
+  property: descriptor({value: "Error"}),
+  afterTest: `trapDidRun === false`,
+}, {
+  // Cross-origin Window objects do expose some properties and have a preview.
+  string: "[object Object]",
+  code: `var obj = Services.appShell.createWindowlessBrowser().document.defaultView;`,
+  hasOwnPropertyNames: true,
+  hasOwnPropertySymbols: true,
+  property: descriptor({value: "SecurityError"}),
+}, {
+  // Cross-origin Location objects do expose some properties and have a preview.
+  string: "[object Object]",
+  code: `var obj = Services.appShell.createWindowlessBrowser().document.defaultView
+                   .location;`,
+  hasOwnPropertyNames: true,
+  hasOwnPropertySymbols: true,
+  property: descriptor({value: "SecurityError"}),
+}];
+
+function descriptor(descr) {
+  return Object.assign({
+    configurable: false,
+    writable: false,
+    enumerable: false,
+    value: undefined
+  }, descr);
+}
+
+async function test_unsafe_grips(principal, tests) {
+  for (let data of tests) {
+    await new Promise(function (resolve) {
+      gThreadClient.addOneTimeListener("paused", async function (event, packet) {
+        let [objGrip, inheritsGrip] = packet.frame.arguments;
+        for (let grip of [objGrip, inheritsGrip]) {
+          let isUnsafe = grip === objGrip;
+          // If `isUnsafe` is true, the parameters in `data` will be used to assert
+          // against `objGrip`, the grip of the object `obj` created by the test.
+          // Otherwise, the grip will refer to `inherits`, an ordinary object which
+          // inherits from `obj`. Then all checks are hardcoded because in every test
+          // all methods are expected to work the same on `inheritsGrip`.
+
+          check_grip(grip, data, isUnsafe);
+
+          let objClient = gThreadClient.pauseGrip(grip);
+          let response, slice;
+
+          response = await objClient.getPrototypeAndProperties();
+          check_properties(response.ownProperties, data, isUnsafe);
+          check_symbols(response.ownSymbols, data, isUnsafe);
+          check_prototype(response.prototype, data, isUnsafe);
+
+          response = await objClient.enumProperties({ignoreIndexedProperties: true});
+          slice = await response.iterator.slice(0, response.iterator.count);
+          check_properties(slice.ownProperties, data, isUnsafe);
+
+          response = await objClient.enumProperties({});
+          slice = await response.iterator.slice(0, response.iterator.count);
+          check_properties(slice.ownProperties, data, isUnsafe);
+
+          response = await objClient.getOwnPropertyNames();
+          check_property_names(response.ownPropertyNames, data, isUnsafe);
+
+          response = await objClient.getProperty("x");
+          check_property(response.descriptor, data, isUnsafe);
+
+          response = await objClient.enumSymbols();
+          slice = await response.iterator.slice(0, response.iterator.count);
+          check_symbol_names(slice.ownSymbols, data, isUnsafe);
+
+          response = await objClient.getProperty(Symbol.for("x"));
+          check_symbol(response.descriptor, data, isUnsafe);
+
+          response = await objClient.getPrototype();
+          check_prototype(response.prototype, data, isUnsafe);
+
+          response = await objClient.getDisplayString();
+          check_display_string(response.displayString, data, isUnsafe);
+
+          if (data.isFunction && isUnsafe) {
+            // For function-related methods, object-client.js checks that the class
+            // of the grip is "Function", and if it's not, the method in object.js
+            // is not called. But some tests have a grip with a class that is not
+            // "Function" (e.g. it's "Proxy") but the DebuggerObject has a "Function"
+            // class because the object is callable (despite not being a Function object).
+            // So the grip class is changed in order to test the object.js method.
+            grip.class = "Function";
+            objClient = gThreadClient.pauseGrip(grip);
+            try {
+              response = await objClient.getParameterNames();
+              ok(true, "getParameterNames passed. DebuggerObject.class is 'Function'"
+                + "on the object actor");
+            } catch(e) {
+              ok(false, "getParameterNames failed. DebuggerObject.class may not be"
+                + " 'Function' on the object actor");
+            }
+          }
+        }
+
+        await gThreadClient.resume();
+        resolve();
+      });
+
+      data = {...defaults, ...data};
+
+      // Run the code and test the results.
+      let sandbox = Cu.Sandbox(systemPrincipal);
+      Object.assign(sandbox, {Services, systemPrincipal, Cu});
+      sandbox.eval(data.code);
+      gDebuggee.obj = sandbox.obj;
+      let inherits = `Object.create(obj, {
+        x: {value: 1},
+        [Symbol.for("x")]: {value: 2}
+      })`;
+      gDebuggee.eval(`stopMe(obj, ${inherits});`);
+      ok(sandbox.eval(data.afterTest), "Check after test passes");
+    });
+  }
+}
+
+function check_grip(grip, data, isUnsafe) {
+  if (isUnsafe) {
+    strictEqual(grip.class, data.class, "The grip has the proper class.");
+    strictEqual("preview" in grip, data.hasPreview, "Check preview presence.");
+  } else {
+    strictEqual(grip.class, "Object", "The grip has 'Object' class.");
+    ok("preview" in grip, "The grip has a preview.");
+  }
+}
+
+function check_properties(props, data, isUnsafe) {
+  let propNames = Reflect.ownKeys(props);
+  check_property_names(propNames, data, isUnsafe);
+  if (isUnsafe) {
+    deepEqual(props.x, undefined, "The property does not exist.");
+  } else {
+    strictEqual(props.x.value, 1, "The property has the right value.");
+  }
+}
+
+function check_property_names(props, data, isUnsafe) {
+  if (isUnsafe) {
+    strictEqual(props.length > 0, data.hasOwnPropertyNames,
+                "Check presence of own string properties.");
+  } else {
+    strictEqual(props.length, 1, "1 own property was retrieved.");
+    strictEqual(props[0], "x", "The property has the right name.");
+  }
+}
+
+function check_property(descr, data, isUnsafe) {
+  if (isUnsafe) {
+    deepEqual(descr, data.property, "Got the right property descriptor.");
+  } else {
+    strictEqual(descr.value, 1, "The property has the right value.");
+  }
+}
+
+function check_symbols(symbols, data, isUnsafe) {
+  check_symbol_names(symbols, data, isUnsafe);
+  if (!isUnsafe) {
+    check_symbol(symbols[0].descriptor, data, isUnsafe);
+  }
+}
+
+function check_symbol_names(props, data, isUnsafe) {
+  if (isUnsafe) {
+    strictEqual(props.length > 0, data.hasOwnPropertySymbols,
+                "Check presence of own symbol properties.");
+  } else {
+    strictEqual(props.length, 1, "1 own symbol property was retrieved.");
+    strictEqual(props[0].name, "Symbol(x)", "The symbol has the right name.");
+  }
+}
+
+function check_symbol(descr, data, isUnsafe) {
+  if (isUnsafe) {
+    deepEqual(descr, data.property, "Got the right symbol property descriptor.");
+  } else {
+    strictEqual(descr.value, 2, "The symbol property has the right value.");
+  }
+}
+
+function check_prototype(proto, data, isUnsafe) {
+  if (isUnsafe) {
+    deepEqual(proto.type, data.protoType, "Got the right prototype type.");
+  } else {
+    check_grip(proto, data, true);
+  }
+}
+
+function check_display_string(str, data, isUnsafe) {
+  if (isUnsafe) {
+    strictEqual(str, data.string, "The object stringifies correctly.");
+  } else {
+    strictEqual(str, "[object Object]", "The object stringifies correctly.");
+  }
+}

devtools/server/tests/unit/xpcshell.ini

@@ -177,6 +177,7 @@ reason = only ran on B2G
 [test_objectgrips-18.js]
 [test_objectgrips-19.js]
 [test_objectgrips-20.js]
+[test_objectgrips-21.js]
 [test_objectgrips-array-like-object.js]
 [test_promise_state-01.js]
 [test_promise_state-02.js]

devtools/shared/DevToolsUtils.js

@@ -184,33 +184,47 @@ exports.defineLazyPrototypeGetter = function (object, key, callback) {
  */
 exports.getProperty = function (object, key) {
   let root = object;
-  try {
-    do {
-      const desc = object.getOwnPropertyDescriptor(key);
-      if (desc) {
-        if ("value" in desc) {
-          return desc.value;
-        }
-        // Call the getter if it's safe.
-        return exports.hasSafeGetter(desc) ? desc.get.call(root).return : undefined;
+  while (object && exports.isSafeDebuggerObject(object)) {
+    let desc;
+    try {
+      desc = object.getOwnPropertyDescriptor(key);
+    } catch (e) {
+      // The above can throw when the debuggee does not subsume the object's
+      // compartment, or for some WrappedNatives like Cu.Sandbox.
+      return undefined;
+    }
+    if (desc) {
+      if ("value" in desc) {
+        return desc.value;
       }
-      object = object.proto;
-    } while (object);
-  } catch (e) {
-    // If anything goes wrong report the error and return undefined.
-    exports.reportException("getProperty", e);
+      // Call the getter if it's safe.
+      if (exports.hasSafeGetter(desc)) {
+        try {
+          return desc.get.call(root).return;
+        } catch (e) {
+          // If anything goes wrong report the error and return undefined.
+          exports.reportException("getProperty", e);
+        }
+      }
+      return undefined;
+    }
+    object = object.proto;
   }
   return undefined;
 };
 
 /**
  * Removes all the non-opaque security wrappers of a debuggee object.
- * Returns null if some wrapper can't be removed.
  *
  * @param obj Debugger.Object
  *        The debuggee object to be unwrapped.
- * @return Debugger.Object|null
- *        The unwrapped object, or null if some wrapper couldn't be removed.
+ * @return Debugger.Object|null|undefined
+ *      - If the object has no wrapper, the same `obj` is returned. Note DeadObject
+ *        objects belong to this case.
+ *      - Otherwise, if the debuggee doesn't subsume object's compartment, returns `null`.
+ *      - Otherwise, if the object belongs to an invisible-to-debugger compartment,
+ *        returns `undefined`. Note CPOW objects belong to this case.
+ *      - Otherwise, returns the unwrapped object.
  */
 exports.unwrap = function unwrap(obj) {
   // Check if `obj` has an opaque wrapper.
@@ -218,12 +232,16 @@ exports.unwrap = function unwrap(obj) {
     return obj;
   }
 
-  // Attempt to unwrap. If this operation is not allowed, it may return null or throw.
+  // Attempt to unwrap via `obj.unwrap()`. Note that:
+  // - This will return `null` if the debuggee does not subsume object's compartment.
+  // - This will throw if the object belongs to an invisible-to-debugger compartment.
+  //   This case includes CPOWs (see bug 1391449).
+  // - This will return `obj` if there is no wrapper.
   let unwrapped;
   try {
     unwrapped = obj.unwrap();
   } catch (err) {
-    unwrapped = null;
+    return undefined;
   }
 
   // Check if further unwrapping is not possible.
@@ -235,6 +253,42 @@ exports.unwrap = function unwrap(obj) {
   return unwrap(unwrapped);
 };
 
+/**
+ * Checks whether a debuggee object is safe. Unsafe objects may run proxy traps or throw
+ * when using `proto`, `isExtensible`, `isFrozen` or `isSealed`. Note that safe objects
+ * may still throw when calling `getOwnPropertyNames`, `getOwnPropertyDescriptor`, etc.
+ * Also note CPOW objects are considered to be unsafe, and DeadObject objects to be safe.
+ *
+ * @param obj Debugger.Object
+ *        The debuggee object to be checked.
+ * @return boolean
+ */
+exports.isSafeDebuggerObject = function (obj) {
+  let unwrapped = exports.unwrap(obj);
+
+  // Objects belonging to an invisible-to-debugger compartment might be proxies,
+  // so just in case consider them unsafe. CPOWs are included in this case.
+  if (unwrapped === undefined) {
+    return false;
+  }
+
+  // If the debuggee does not subsume the object's compartment, most properties won't
+  // be accessible. Cross-origin Window and Location objects might expose some, though.
+  // Therefore, it must be considered safe. Note that proxy objects have fully opaque
+  // security wrappers, so proxy traps won't run in this case.
+  if (unwrapped === null) {
+    return true;
+  }
+
+  // Proxy objects can run traps when accessed. `isProxy` getter is called on `unwrapped`
+  // instead of on `obj` in order to detect proxies behind transparent wrappers.
+  if (unwrapped.isProxy) {
+    return false;
+  }
+
+  return true;
+};
+
 /**
  * Determines if a descriptor has a getter which doesn't call into JavaScript.
  *

devtools/shared/webconsole/test/test_consoleapi.html

@@ -149,7 +149,7 @@ function doConsoleCalls(aState)
         {
           type: "object",
           actor: /[a-z]/,
-          class: "Inaccessible",
+          class: "InvisibleToDebugger: Object",
         },
       ],
     },