mozilla / positron

a experimental, Electron-compatible runtime on top of Gecko
Other
564 stars 64 forks source link

figure out how to make cross-sandbox Object.prototype comparisons #45

Open mykmelez opened 8 years ago

mykmelez commented 8 years ago

https://github.com/mozilla/positron/blob/master/positron/electron/lib/browser/rpc-server.js#L46 checks if a prototype object is strictly equal to Object.prototype, i.e. proto === Object.prototype. But the prototype can come from a different sandbox than the one in which the comparison is taking place, in which case the condition evaluates to false, even if proto is an Object.prototype, perhaps because each sandbox gets its own unique instance of Object, so it isn't the same Object.protoype.

In #26, I work around the problem with a duck-type comparison. But that requires changing the code, which comes from upstream Electron. Ideally, we wouldn't have to modify the Electron code in order to ensure that conditionals like this one behave as it expects.

mykmelez commented 8 years ago

@khuey do you have insight here into how sandboxes behave, and how we might configure them to have the behavior we desire?

Note that there isn't a security concern here. Both sandboxes are fully privileged and are loading modules that belong to the app itself. So giving them the same Object global wouldn't introduce a vulnerability. And that may be what Node does (unsure).

mykmelez commented 8 years ago

Note that there isn't a security concern here. Both sandboxes are fully privileged and are loading modules that belong to the app itself. So giving them the same Object global wouldn't introduce a vulnerability. And that may be what Node does (unsure).

Or rather: I don't think there's a security concern. But I'm happy to be disabused of my naiveté. ;-)

khuey commented 8 years ago

I don't know much about sandboxes ... that's more of a @bholley question.

bholley commented 8 years ago

Can you clarify a bit more what's going on here? Does Eletron run code in one global Positron we run it in two separate ones?

Separate prototypes are not unique to sandboxes and are totally something that happens on the web (i.e. same-origin iframes).

mykmelez commented 8 years ago

Can you clarify a bit more what's going on here?

Sure!

Does Eletron run code in one global Positron we run it in two separate ones?

Electron uses Node to load modules, and Node loads them in a way that shares globals (or at least appears to do so). For example, given a Node app with two modules:

foo.js:

let bar = require("./bar"); // import the "bar" object from bar.js
// Is bar's prototype identical to the Object.prototype in this module?
console.log(Object.getPrototypeOf(bar) === Object.prototype);

bar.js:

module.exports = {};

If I run this app in Node, it outputs "true". But the Node-like module loader in Positron, which loads each module inside its own sandbox, would output "false". And that breaks an Electron module we've imported into Positron, since it counts on the behavior in Node.

mykmelez commented 8 years ago

Another way of asking this:

Is there a way to maintain the identity of built-in objects like Object across compartment boundaries, such that Cu.import("resource://gre/modules/Services.jsm", {}).Object === Object?

Alternately:

Is there a way to evaluate scripts in the same compartment, such that they share built-in objects like Object, without them sharing a global scope?

mykmelez commented 8 years ago

In case it helps, the actual code does something like this:

const systemPrincipal = Cc["@mozilla.org/systemprincipal;1"].
                        createInstance(Ci.nsIPrincipal);
const subScriptLoader = Cc['@mozilla.org/moz/jssubscript-loader;1'].
                        getService(Ci.mozIJSSubScriptLoader);
let sandboxA = new Cu.Sandbox(systemPrincipal);
subScriptLoader.loadSubScript(scriptA, sandbox, "UTF-8");
let sandboxB = new Cu.Sandbox(systemPrincipal);
subScriptLoader.loadSubScript(scriptB, sandbox, "UTF-8");
sandboxA.Object === sandboxB.Object; // I want this to evaluate to `true`.
mykmelez commented 8 years ago

Update @bholley and I chatted in IRC, where he asked some probing questions that caused me to investigate Node's behavior further, and it turns out that Node actually reuses the same global for all modules and evaluates them as closures, such that foo = 1 and (new Function('bar = 2;'))() both create shared globals, while var baz = 3 creates a module-specific global.

Thus we should be able to replicate Node's behavior by using loadSubScript to load each module as a closure with a different targetObj but in the same sandbox, thus giving them the same global (and its built-ins).

@bholley did have some concern about the JIT perf consequences of calling loadSubScript with an explicit targetObj, since https://bugzilla.mozilla.org/show_bug.cgi?id=1097987 changed the way that works. I'm following up with @jandem about that in #jsapi.

brendandahl commented 8 years ago

Looks like the failure comes from https://github.com/mozilla/gecko-dev/blob/a8add2ef97e68e80eef21fa8bc78951207ed5883/dom/bindings/BindingUtils.h#L206 still looking at ways around it.