Bug 796073: Identify debuggees only by Debugger.Object instances and CCWs that can be unwrapped securely. r=luke
authorJim Blandy <jimb@mozilla.com>
Sat, 13 Oct 2012 16:04:41 -0700
changeset 118139 19abbe2af7cfe9cab121785c975c8996750c107d
parent 118138 567352805e77069675ac063ae2ff38ad2e3a9499
child 118140 10f0632888bb4086cda39330fd574ed2749c02b9
push id1997
push userakeybl@mozilla.com
push dateMon, 07 Jan 2013 21:25:26 +0000
treeherdermozilla-beta@4baf45cdcf21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs796073
milestone19.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 796073: Identify debuggees only by Debugger.Object instances and CCWs that can be unwrapped securely. r=luke This patch affects behavior that a lot of debugger tests rely on; I've tried to update the tests without losing coverage of behavior that is still supported: - A prior patch in this series removes uses of addDebuggee to generate Debugger.Object instances referring to random objects, using makeDebuggeeValue instead. - The test debug/Debugger-debuggees-07.js is deleted, because it's testing for the very behavior we're removing. Other tests are trimmed to remove usage that is no longer supported. - The new test debug/Debugger-debuggees-17.js checks that we reject objects that don't designate debuggees. The existing test Debugger-debuggees-06.js checks that non-object values are properly rejected. - The new test debug/Debugger-debuggees-18.js checks that globals are correctly identified regardless of how we designate them.
js/src/jit-test/tests/debug/Debugger-debuggees-07.js
js/src/jit-test/tests/debug/Debugger-debuggees-08.js
js/src/jit-test/tests/debug/Debugger-debuggees-09.js
js/src/jit-test/tests/debug/Debugger-debuggees-17.js
js/src/jit-test/tests/debug/Debugger-debuggees-18.js
js/src/jit-test/tests/debug/Debugger-findScripts-09.js
js/src/vm/Debugger.cpp
js/src/vm/Debugger.h
deleted file mode 100644
--- a/js/src/jit-test/tests/debug/Debugger-debuggees-07.js
+++ /dev/null
@@ -1,24 +0,0 @@
-// Handle proto-less objects passed to addDebuggee.
-
-var g = newGlobal('new-compartment');
-var obj = g.eval("Object.create(null)");
-var dbg = new Debugger;
-
-// hasDebuggee and removeDebuggee must tolerate proto-less objects.
-assertEq(dbg.hasDebuggee(obj), false);
-
-// addDebuggee may either succeed or throw a TypeError. Don't care.
-var added;
-try {
-    dbg.addDebuggee(obj);
-    added = true;
-} catch (exc) {
-    if (!(exc instanceof TypeError))
-        throw exc;
-    added = false;
-}
-
-assertEq(dbg.hasDebuggee(obj), added);
-assertEq(dbg.getDebuggees().length, added ? 1 : 0);
-assertEq(dbg.removeDebuggee(obj), undefined);
-assertEq(dbg.hasDebuggee(obj), false);
--- a/js/src/jit-test/tests/debug/Debugger-debuggees-08.js
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-08.js
@@ -1,9 +1,8 @@
-// addDebuggee(obj), where obj is not global, adds obj's global.
 // Adding a debuggee more than once is redundant.
 
 var dbg = new Debugger;
 var g = newGlobal('new-compartment');
 var w = dbg.addDebuggee(g);
 assertEq(w instanceof Debugger.Object, true);
 
 function usual() {
@@ -14,25 +13,13 @@ function usual() {
     assertEq(arr[0], w);
 }
 
 usual();
 assertEq(dbg.addDebuggee(g), w);
 usual();
 assertEq(dbg.addDebuggee(w), w);
 usual();
-dbg.addDebuggee(g.Math);
-usual();
-dbg.addDebuggee(g.eval("(function () {})"));
-usual();
-
-// w2 is a Debugger.Object in g. Treat it like any other object in g; don't auto-unwrap it.
-g.g2 = newGlobal('new-compartment');
-g.eval("var w2 = new Debugger().addDebuggee(g2)");
-dbg.addDebuggee(g.w2);
-usual();
-assertEq(!dbg.hasDebuggee(g.g2), true);
-assertEq(dbg.hasDebuggee(g.w2), true);
 
 // Removing the debuggee once is enough.
 assertEq(dbg.removeDebuggee(g), undefined);
 assertEq(dbg.hasDebuggee(g), false);
 assertEq(dbg.getDebuggees().length, 0);
--- a/js/src/jit-test/tests/debug/Debugger-debuggees-09.js
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-09.js
@@ -1,27 +1,22 @@
 // |jit-test| debug
-// Random objects can be the argument to hasDebuggee and removeDebuggee.
 // If hasDebuggee(x) is false, removeDebuggee(x) does nothing.
 
 var dbg = new Debugger;
 
 function check(obj) {
     // If obj is something we could never debug, hasDebuggee(obj) is false.
     assertEq(dbg.hasDebuggee(obj), false);
 
     // If hasDebuggee(x) is false, removeDebuggee(x) does nothing.
     assertEq(dbg.removeDebuggee(obj), undefined);
 }
 
-// objects in this compartment, which we can't debug
-check(this);
-check({});
+// global objects which happen not to be debuggees at the moment
 var g1 = newGlobal('same-compartment');
 check(g1);
-check(g1.eval("({})"));
 
 // objects in a compartment that is already debugging us
 var g2 = newGlobal('new-compartment');
 g2.parent = this;
 g2.eval("var dbg = new Debugger(parent);");
 check(g2);
-check(g2.dbg);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-17.js
@@ -0,0 +1,26 @@
+// addDebuggee, hasDebuggee, and removeDebuggee all throw if presented with
+// objects that are not valid global object designators.
+
+load(libdir + 'asserts.js');
+
+var dbg = new Debugger;
+
+function check(bad) {
+  print("check(" + uneval(bad) + ")");
+  assertThrowsInstanceOf(function () { dbg.addDebuggee(bad); }, TypeError);
+  assertEq(dbg.getDebuggees().length, 0);
+  assertThrowsInstanceOf(function () { dbg.hasDebuggee(bad); }, TypeError);
+  assertThrowsInstanceOf(function () { dbg.removeDebuggee(bad); }, TypeError);
+}
+
+var g = newGlobal();
+check(g.Object());
+check(g.Object);
+check(g.Function(""));
+
+// A Debugger.Object belonging to a different Debugger is not a valid way
+// to designate a global, even if its referent is a global.
+var g2 = newGlobal();
+var dbg2 = new Debugger;
+var d2g2 = dbg2.addDebuggee(g2);
+check(d2g2);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-18.js
@@ -0,0 +1,117 @@
+// Debugger.prototype.{addDebuggee,hasDebuggee,removeDebuggee} recognize globals
+// regardless of how they are specified.
+
+var dbg = new Debugger;
+
+// Assert that dbg's debuggees are exactly the set passed as arguments.
+// The arguments are assumed to be Debugger.Object instances referring to
+// globals without wrappers --- which is the sort returned by addDebuggee.
+function assertDebuggees() {
+  print("assertDebuggees([" + [g.toSource() for each (g in arguments)] + "])");
+  var debuggees = dbg.getDebuggees();
+  assertEq(arguments.length, debuggees.length);
+  for each (g in arguments)
+    assertEq(debuggees.indexOf(g) != -1, true);
+}
+
+var g1 = newGlobal(); g1.toSource = function () { return "[global g1]"; };
+var g2 = newGlobal(); g2.toSource = function () { return "[global g2]"; };
+
+assertDebuggees();
+
+// Produce every possible way to designate g1, for us to play with.
+// Globals can be designated by any of the following:
+//
+// - "CCW": a Cross-Compartment Wrapper (CCW) of a global object
+// - "D.O": a Debugger.Object whose referent is a global object
+// - "D.O of CCW": a Debugger.Object whose referent is a CCW of a
+//   global object, where the CCW can be securely unwrapped
+//
+// There's no direct "G", since globals are always in their own
+// compartments, never the debugger's; if we ever viewed them directly,
+// that would be a compartment violation.
+
+// "dg1" means "Debugger.Object referring (directly) to g1".
+var dg1 = dbg.addDebuggee(g1);
+dg1.toSource = function() { return "[Debugger.Object for global g1]"; };
+assertEq(dg1.global, dg1);
+assertEq(dg1.unwrap(), dg1);
+assertDebuggees(dg1);
+
+// We need to add g2 as a debuggee; that's the only way to get a D.O referring
+// to it without a wrapper.
+var dg2 = dbg.addDebuggee(g2);
+dg2.toSource = function() { return "[Debugger.Object for global g2]"; };
+assertEq(dg2.global, dg2);
+assertEq(dg2.unwrap(), dg2);
+assertDebuggees(dg1, dg2);
+
+// "dwg1" means "Debugger.Object referring to CCW of g1".
+var dwg1 = dg2.makeDebuggeeValue(g1);
+assertEq(dwg1.global, dg2);
+assertEq(dwg1.unwrap(), dg1);
+dwg1.toSource = function() { return "[Debugger.Object for CCW of global g1]"; };
+
+assertDebuggees(dg1, dg2);
+assertEq(dbg.removeDebuggee(g1), undefined);
+assertEq(dbg.removeDebuggee(g2), undefined);
+assertDebuggees();
+
+// Systematically cover all the single-global possibilities:
+//
+//  | added as    | designated as | addDebuggee | hasDebuggee | removeDebuggee |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | (not added) | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | CCW         | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | D.O         | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+//  |-------------+---------------+-------------+-------------+----------------|
+//  | D.O of CCW  | CCW           | X           | X           | X              |
+//  |             | D.O           | X           | X           | X              |
+//  |             | D.O of CCW    | X           | X           | X              |
+
+// Cover the "(not added)" section of the table, other than "addDebuggee":
+assertEq(dbg.hasDebuggee(g1), false);
+assertEq(dbg.hasDebuggee(dg1), false);
+assertEq(dbg.hasDebuggee(dwg1), false);
+
+assertEq(dbg.removeDebuggee(g1), undefined); assertDebuggees();
+assertEq(dbg.removeDebuggee(dg1), undefined); assertDebuggees();
+assertEq(dbg.removeDebuggee(dwg1), undefined); assertDebuggees();
+
+// Try all operations adding the debuggee using |addAs|, and operating on it
+// using |designateAs|, thereby covering one row of the table (outside the '(not
+// added)' section), and one case in the '(not added)', 'designated as' section.
+//
+// |Direct| should be the Debugger.Object referring directly to the debuggee
+// global, for checking the results from addDebuggee and getDebuggees.
+function combo(addAs, designateAs, direct) {
+  print("combo(" + uneval(addAs) + ", " + uneval(designateAs) + ")");
+  assertDebuggees();
+  assertEq(dbg.addDebuggee(addAs), direct);
+  assertDebuggees(direct);
+  assertEq(dbg.addDebuggee(designateAs), direct);
+  assertDebuggees(direct);
+  assertEq(dbg.hasDebuggee(designateAs), true);
+  assertEq(dbg.removeDebuggee(designateAs), undefined);
+  assertDebuggees();
+}
+
+combo(g1, g1, dg1);
+combo(dg1, g1, dg1);
+combo(dwg1, g1, dg1);
+
+combo(g1, dg1, dg1);
+combo(dg1, dg1, dg1);
+combo(dwg1, dg1, dg1);
+
+combo(g1, dwg1, dg1);
+combo(dg1, dwg1, dg1);
+combo(dwg1, dwg1, dg1);
--- a/js/src/jit-test/tests/debug/Debugger-findScripts-09.js
+++ b/js/src/jit-test/tests/debug/Debugger-findScripts-09.js
@@ -1,16 +1,17 @@
 // Passing bad query properties to Debugger.prototype.findScripts throws.
 load(libdir + 'asserts.js');
 
 var dbg = new Debugger();
+var g = newGlobal();
 assertEq(dbg.findScripts().length, 0);
 assertEq(dbg.findScripts({}).length, 0);
 
-assertEq(dbg.findScripts({global:{}}).length, 0);
+assertEq(dbg.findScripts({global:g}).length, 0);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:null}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:true}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:4}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({global:"I must have fruit!"}); }, TypeError);
 
 assertEq(dbg.findScripts({url:""}).length, 0);
 assertThrowsInstanceOf(function () { dbg.findScripts({url:null}); }, TypeError);
 assertThrowsInstanceOf(function () { dbg.findScripts({url:true}); }, TypeError);
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -1657,84 +1657,97 @@ Debugger::setUncaughtExceptionHook(JSCon
         return false;
     }
 
     dbg->uncaughtExceptionHook = args[0].toObjectOrNull();
     args.rval().setUndefined();
     return true;
 }
 
-JSObject *
+GlobalObject *
 Debugger::unwrapDebuggeeArgument(JSContext *cx, const Value &v)
 {
-    /*
-     * The argument to {add,remove,has}Debuggee may be
-     *   - a Debugger.Object belonging to this Debugger: return its referent
-     *   - a cross-compartment wrapper: return the wrapped object
-     *   - any other non-Debugger.Object object: return it
-     * If it is a primitive, or a Debugger.Object that belongs to some other
-     * Debugger, throw a TypeError.
-     */
-    JSObject *obj = NonNullObject(cx, v);
-    if (obj) {
-        if (obj->getClass() == &DebuggerObject_class) {
-            Value rv = v;
-            if (!unwrapDebuggeeValue(cx, &rv))
-                return NULL;
-            return &rv.toObject();
-        }
-        if (IsCrossCompartmentWrapper(obj))
-            return &GetProxyPrivate(obj).toObject();
+    if (!v.isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_UNEXPECTED_TYPE,
+                             "argument", "not a global object");
+        return NULL;
+    }
+
+    RootedObject obj(cx, &v.toObject());
+
+    /* If it's a Debugger.Object belonging to this debugger, dereference that. */
+    if (obj->getClass() == &DebuggerObject_class) {
+        Value rv = v;
+        if (!unwrapDebuggeeValue(cx, &rv))
+            return NULL;
+        obj = &rv.toObject();
     }
-    return obj;
+
+    /* If we have a cross-compartment wrapper, dereference as far as is secure. */
+    obj = UnwrapObjectChecked(cx, obj);
+    if (!obj)
+        return NULL;
+
+    /* If that produced an outer window, innerize it. */
+    obj = GetInnerObject(cx, obj);
+    if (!obj)
+        return NULL;
+
+    /* If that didn't produce a global object, it's an error. */
+    if (!obj->isGlobal()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_UNEXPECTED_TYPE,
+                             "argument", "not a global object");
+        return NULL;
+    }
+
+    return &obj->asGlobal();
 }
 
 JSBool
 Debugger::addDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.addDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "addDebuggee", args, dbg);
-    RootedObject referent(cx, dbg->unwrapDebuggeeArgument(cx, args[0]));
-    if (!referent)
+    Rooted<GlobalObject*> global(cx, dbg->unwrapDebuggeeArgument(cx, args[0]));
+    if (!global)
         return false;
-    Rooted<GlobalObject*> global(cx, &referent->global());
+
     if (!dbg->addDebuggeeGlobal(cx, global))
         return false;
 
-    Value v = ObjectValue(*referent);
+    Value v = ObjectValue(*global);
     if (!dbg->wrapDebuggeeValue(cx, &v))
         return false;
     args.rval().set(v);
     return true;
 }
 
 JSBool
 Debugger::removeDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.removeDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "removeDebuggee", args, dbg);
-    JSObject *referent = dbg->unwrapDebuggeeArgument(cx, args[0]);
-    if (!referent)
+    GlobalObject *global = dbg->unwrapDebuggeeArgument(cx, args[0]);
+    if (!global)
         return false;
-    GlobalObject *global = &referent->global();
     if (dbg->debuggees.has(global))
         dbg->removeDebuggeeGlobal(cx->runtime->defaultFreeOp(), global, NULL, NULL);
     args.rval().setUndefined();
     return true;
 }
 
 JSBool
 Debugger::hasDebuggee(JSContext *cx, unsigned argc, Value *vp)
 {
     REQUIRE_ARGC("Debugger.hasDebuggee", 1);
     THIS_DEBUGGER(cx, argc, vp, "hasDebuggee", args, dbg);
-    JSObject *referent = dbg->unwrapDebuggeeArgument(cx, args[0]);
-    if (!referent)
+    GlobalObject *global = dbg->unwrapDebuggeeArgument(cx, args[0]);
+    if (!global)
         return false;
-    args.rval().setBoolean(!!dbg->debuggees.lookup(&referent->global()));
+    args.rval().setBoolean(!!dbg->debuggees.lookup(global));
     return true;
 }
 
 JSBool
 Debugger::getDebuggees(JSContext *cx, unsigned argc, Value *vp)
 {
     THIS_DEBUGGER(cx, argc, vp, "getDebuggees", args, dbg);
     RootedObject arrobj(cx, NewDenseAllocatedArray(cx, dbg->debuggees.count()));
@@ -2006,20 +2019,19 @@ class Debugger::ScriptQuery {
          * scripts scoped to a particular global object.
          */
         RootedValue global(cx);
         if (!JSObject::getProperty(cx, query, query, cx->names().global, &global))
             return false;
         if (global.isUndefined()) {
             matchAllDebuggeeGlobals();
         } else {
-            JSObject *referent = debugger->unwrapDebuggeeArgument(cx, global);
-            if (!referent)
+            GlobalObject *globalObject = debugger->unwrapDebuggeeArgument(cx, global);
+            if (!globalObject)
                 return false;
-            GlobalObject *globalObject = &referent->global();
 
             /*
              * If the given global isn't a debuggee, just leave the set of
              * acceptable globals empty; we'll return no scripts.
              */
             if (debugger->debuggees.has(globalObject)) {
                 if (!matchSingleGlobal(globalObject))
                     return false;
--- a/js/src/vm/Debugger.h
+++ b/js/src/vm/Debugger.h
@@ -134,17 +134,17 @@ class Debugger {
      *     null - Return JSTRAP_ERROR to terminate the debuggee with an
      *         uncatchable error.
      *     anything else - Make a new TypeError the pending exception and
      *         return handleUncaughtException(ac, vp, callHook).
      */
     JSTrapStatus parseResumptionValue(Maybe<AutoCompartment> &ac, bool ok, const Value &rv,
                                       Value *vp, bool callHook = true);
 
-    JSObject *unwrapDebuggeeArgument(JSContext *cx, const Value &v);
+    GlobalObject *unwrapDebuggeeArgument(JSContext *cx, const Value &v);
 
     static void traceObject(JSTracer *trc, RawObject obj);
     void trace(JSTracer *trc);
     static void finalize(FreeOp *fop, RawObject obj);
     void markKeysInCompartment(JSTracer *tracer);
 
     static Class jsclass;