Bug 1473957 - Require debugger and debuggee to be in different compartments. r=jimb
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 16 Aug 2018 12:35:20 +0200
changeset 432570 bb155905368517d486de333676962091fb317e54
parent 432569 8695f16b39ed48f002dccabf249da238bdb3e8b4
child 432571 bd62249f8b51bb395883e0115f0cac099555be1c
push id106782
push userjandemooij@gmail.com
push dateTue, 21 Aug 2018 12:06:05 +0000
treeherdermozilla-inbound@bb1559053685 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimb
bugs1473957
milestone63.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 1473957 - Require debugger and debuggee to be in different compartments. r=jimb We were checking for cross-compartment wrappers in the Debugger constructor, but this patch also fixes addDebuggee and addAllGlobalsAsDebuggees. Differential Revision: https://phabricator.services.mozilla.com/D3495
js/src/jit-test/tests/debug/Debugger-debuggees-30.js
js/src/js.msg
js/src/vm/Debugger.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/Debugger-debuggees-30.js
@@ -0,0 +1,32 @@
+// Debugger and debuggees must be in different compartments.
+
+load(libdir + "asserts.js");
+
+function testConstructor() {
+    var g = newGlobal({sameCompartmentAs: this});
+    assertTypeErrorMessage(() => new Debugger(g),
+                           "Debugger: argument must be an object from a different compartment");
+}
+testConstructor();
+
+function testAddDebuggee() {
+    var g = newGlobal({sameCompartmentAs: this});
+    var dbg = new Debugger();
+    assertTypeErrorMessage(() => dbg.addDebuggee(this),
+                           "debugger and debuggee must be in different compartments");
+}
+testAddDebuggee();
+
+function testAddAllGlobalsAsDebuggees() {
+    var g1 = newGlobal({sameCompartmentAs: this});
+    var g2 = newGlobal();
+    var g3 = newGlobal({sameCompartmentAs: g2});
+    var g4 = newGlobal({sameZoneAs: this});
+    var dbg = new Debugger();
+    dbg.addAllGlobalsAsDebuggees();
+    assertEq(dbg.hasDebuggee(g1), false);
+    assertEq(dbg.hasDebuggee(g2), true);
+    assertEq(dbg.hasDebuggee(g3), true);
+    assertEq(dbg.hasDebuggee(g4), true);
+}
+testAddAllGlobalsAsDebuggees();
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -456,16 +456,17 @@ MSG_DEF(JSMSG_SC_SHMEM_POLICY,         0
 // Debugger
 MSG_DEF(JSMSG_ASSIGN_FUNCTION_OR_NULL, 1, JSEXN_TYPEERR, "value assigned to {0} must be a function or null")
 MSG_DEF(JSMSG_DEBUG_BAD_LINE,          0, JSEXN_TYPEERR, "invalid line number")
 MSG_DEF(JSMSG_DEBUG_BAD_OFFSET,        0, JSEXN_TYPEERR, "invalid script offset")
 MSG_DEF(JSMSG_DEBUG_BAD_REFERENT,      2, JSEXN_TYPEERR, "{0} does not refer to {1}")
 MSG_DEF(JSMSG_DEBUG_BAD_RESUMPTION,    0, JSEXN_TYPEERR, "debugger resumption value must be undefined, {throw: val}, {return: val}, or null")
 MSG_DEF(JSMSG_DEBUG_BAD_YIELD,         0, JSEXN_TYPEERR, "generator yielded invalid value")
 MSG_DEF(JSMSG_DEBUG_CANT_DEBUG_GLOBAL, 0, JSEXN_TYPEERR, "passing non-debuggable global to addDebuggee")
+MSG_DEF(JSMSG_DEBUG_SAME_COMPARTMENT,  0, JSEXN_TYPEERR, "debugger and debuggee must be in different compartments")
 MSG_DEF(JSMSG_DEBUG_CCW_REQUIRED,      1, JSEXN_TYPEERR, "{0}: argument must be an object from a different compartment")
 MSG_DEF(JSMSG_DEBUG_COMPARTMENT_MISMATCH, 2, JSEXN_TYPEERR, "{0}: descriptor .{1} property is an object in a different compartment than the target object")
 MSG_DEF(JSMSG_DEBUG_LOOP,              0, JSEXN_TYPEERR, "cannot debug an object in same compartment as debugger or a compartment that is already debugging the debugger")
 MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGEE,      2, JSEXN_ERR, "{0} is not a debuggee {1}")
 MSG_DEF(JSMSG_DEBUG_NOT_DEBUGGING,     0, JSEXN_ERR, "can't set breakpoint: script global is not a debuggee")
 MSG_DEF(JSMSG_DEBUG_NOT_IDLE,          0, JSEXN_ERR, "can't start debugging: a debuggee script is on the stack")
 MSG_DEF(JSMSG_DEBUG_NOT_LIVE,          1, JSEXN_ERR, "{0} is not live")
 MSG_DEF(JSMSG_DEBUG_NO_ENV_OBJECT,     0, JSEXN_TYPEERR, "declarative Environments don't have binding objects")
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -3718,19 +3718,21 @@ Debugger::addDebuggee(JSContext* cx, uns
     args.rval().set(v);
     return true;
 }
 
 /* static */ bool
 Debugger::addAllGlobalsAsDebuggees(JSContext* cx, unsigned argc, Value* vp)
 {
     THIS_DEBUGGER(cx, argc, vp, "addAllGlobalsAsDebuggees", args, dbg);
-    for (ZonesIter zone(cx->runtime(), SkipAtoms); !zone.done(); zone.next()) {
-        for (RealmsInZoneIter r(zone); !r.done(); r.next()) {
-            if (r == dbg->object->realm() || r->creationOptions().invisibleToDebugger())
+    for (CompartmentsIter comp(cx->runtime()); !comp.done(); comp.next()) {
+        if (comp == dbg->object->compartment())
+            continue;
+        for (RealmsInCompartmentIter r(comp); !r.done(); r.next()) {
+            if (r->creationOptions().invisibleToDebugger())
                 continue;
             r->compartment()->gcState.scheduledForDestruction = false;
             GlobalObject* global = r->maybeGlobal();
             if (global) {
                 Rooted<GlobalObject*> rg(cx, global);
                 if (!dbg->addDebuggeeGlobal(cx, rg))
                     return false;
             }
@@ -3916,17 +3918,17 @@ Debugger::construct(JSContext* cx, unsig
 
         debugger = dbg.release();
         obj->setPrivate(debugger); // owns the released pointer
     }
 
     // Add the initial debuggees, if any.
     for (unsigned i = 0; i < args.length(); i++) {
         JSObject& wrappedObj = args[i].toObject().as<ProxyObject>().private_().toObject();
-        Rooted<GlobalObject*> debuggee(cx, &wrappedObj.deprecatedGlobal());
+        Rooted<GlobalObject*> debuggee(cx, &wrappedObj.nonCCWGlobal());
         if (!debugger->addDebuggeeGlobal(cx, debuggee))
             return false;
     }
 
     args.rval().setObject(*obj);
     return true;
 }
 
@@ -3941,16 +3943,22 @@ Debugger::addDebuggeeGlobal(JSContext* c
     // with certain testing aides we expose in the shell, so just make addDebuggee
     // throw in that case.
     Realm* debuggeeRealm = global->realm();
     if (debuggeeRealm->creationOptions().invisibleToDebugger()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_CANT_DEBUG_GLOBAL);
         return false;
     }
 
+    // Debugger and debuggee must be in different compartments.
+    if (debuggeeRealm->compartment() == object->compartment()) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_SAME_COMPARTMENT);
+        return false;
+    }
+
     // Check for cycles. If global's realm is reachable from this Debugger
     // object's realm by following debuggee-to-debugger links, then adding
     // global would create a cycle. (Typically nobody is debugging the
     // debugger, in which case we zip through this code without looping.)
     Vector<Realm*> visited(cx);
     if (!visited.append(object->realm()))
         return false;
     for (size_t i = 0; i < visited.length(); i++) {