Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polyfilling or virtualizing unbox #254

Closed
caridy opened this issue Sep 19, 2021 · 28 comments
Closed

polyfilling or virtualizing unbox #254

caridy opened this issue Sep 19, 2021 · 28 comments
Labels
boxes All the discussions related to built-in object placeholders

Comments

@caridy
Copy link

caridy commented Sep 19, 2021

An interesting aspect here is how to polyfill or virtualize unbox? can it be replace? can it be patched? I'm still confused about how unbox gets into the record without breaking the invariants of a record. Obviously, that will not be the case if you have to use Record.unbox(someRecord) instead of the dot notation on the record.

@acutmore
Copy link
Collaborator

Box is pollyfilled as part of the R&T pollyfill: https://github.com/bloomberg/record-tuple-polyfill

I don’t think it can be pollyfilled on its own without symbols-as-weakMap-keys, and adding unbox to the Symbol.prototype. Making all symbols appear to be boxes.

I'm still confused about how unbox gets into the record without breaking the invariants of a record

Unbox doesn’t go on Record.prototype. Box is a separate immutable type.

let r = #{ a: 1, b: Box(function(){}) }

let f = r.b.unbox();

@caridy caridy closed this as completed Sep 22, 2021
@mhofman
Copy link
Member

mhofman commented Sep 30, 2021

The virtualizing question is still valid. For example, to make a compartment specific version of Box and Box.unbox, you could do:

const compartmentMap = new WeakMap();

const OriginalBox = globalThis.Box;

globalThis.Box = function Box(target) {
  let wrappedTarget = compartmentMap.get(target);
  if (!wrappedTarget) {
    wrappedTarget = Object.freeze({target});
    compartmentMap.set(target, wrappedTarget);
  }
  return OriginalBox(wrappedTarget);
};

globalThis.Box.unbox = function unbox(box) {
  const wrappedTarget = OriginalBox.unbox(box);
  if (compartmentMap.get(wrappedTarget.target) !== wrappedTarget) {
    throw new TypeError();
  }
  return wrappedTarget.target;
};

This would replicate the proposed realm semantics where the box for a given target created in one compartment isn't equal to the one created in another compartment, preventing unboxing unless you have access to the other compartment's unbox.

cc @erights

@erights
Copy link

erights commented Sep 30, 2021

The additional wiring to follow the inert constructor pattern:

defineProperties(GlobalThis.Box, {
  prototype: {
    value: OriginalBox.prototype
  }
});

defineProperties(OriginalBox.prototype, {
  constructor: {
    value: function InertBox(target) {
      throw TypeError('Try boxing with some globalThis.Box');
    }
  }
});

Then

box = anyGlobal.Box(target);
box instanceof anyOtherGlobal.Box; // true

@mhofman
Copy link
Member

mhofman commented Oct 1, 2021

The additional wiring to follow the inert constructor pattern:

Right, I only wrote the relevant code, not the rest of plumbing.

@nicolo-ribaudo can we re-open to discuss Compartment virtualization of Box ?

Following up on #257 (review), I think we can make Box::toString and JSON.stringify be compatible with virtualization even if they implicitly unbox, as long as they immediately (without other user code interleaving) respectively call toString() and toJSON() on the boxed content.

This would make boxes transparent for serialization, while preserving virtualization.

Adapting the previous code (and now allowing primitives to follow latest changes):

// Global taming
const OriginalBox = globalThis.Box;

defineProperties(OriginalBox.prototype, {
  constructor: {
    value: function InertBox(target) {
      throw TypeError('Try boxing with some globalThis.Box');
    }
  }
});

// Unique Box per compartment
const compartmentMap = new WeakMap();

function unwrap(wrappedTarget) {
  if (compartmentMap.get(wrappedTarget.target) !== wrappedTarget) {
    throw new TypeError();
  }
  return wrappedTarget.target;
}

function unbox(box) {
  const maybeWrappedTarget = OriginalBox.unbox(box);
  if (Object(maybeWrappedTarget) === maybeWrappedTarget) {
    return unwrap(maybeWrappedTarget);
  }
  return maybeWrappedTarget;
}

function toString() {
  const target = unwrap(this);
  // Perform https://tc39.es/ecma262/#sec-ordinarytoprimitive
  for (const methodName of ['toString', 'valueOf']) {
    const method = target[methodName];
    if (typeof method === 'function') {
      const result = method.call(target);
      if (Object(result) !== result) {
        return result;
      }
    }
  }
  throw new TypeError();
}

function toJSON(key) {
  const target = unwrap(this);
  const targetToJSON = target.toJSON;
  if (typeof targetToJSON === 'function') {
    return targetToJSON.call(target, key);
  }
  return target;
}

function Box(target) {
  if (Object(target) !== target) {
    return OriginalBox(target);
  }

  let wrappedTarget = compartmentMap.get(target);
  if (!wrappedTarget) {
    wrappedTarget = Object.freeze({
      target,
      toString,
      toJSON,
    });
    compartmentMap.set(target, wrappedTarget);
  }
  return OriginalBox(wrappedTarget);
}

globalThis.Box = Box;

defineProperties(globalThis.Box, {
  prototype: {
    value: OriginalBox.prototype
  },
  unbox: {
    value: unbox
  }
});

@acutmore
Copy link
Collaborator

acutmore commented Oct 1, 2021

Following up on #257 (review), I think we can make Box::toString and JSON.stringify be compatible with virtualization even if they implicitly unbox, as long as they immediately (without other user code interleaving) respectively call toString() and toJSON() on the boxed content.

Are we saying JSON.stringify can unbox (even across realms) as long as any value that was reached while recusing past an automatic unboxing boundary the replacer is no longer called? When encountering a box stringify can still first pass the box to the replacer, so the replacer can always perform unboxing on behalf of JSON.stringify, and this would not be treated as an 'auto-unbox', but if the replacer returned a box and this is then auto-unboxed, the remaining subtree can not be passed to the replacer.

@acutmore
Copy link
Collaborator

acutmore commented Oct 1, 2021

At least I know understand that there is a difference between polyfilling and virtualizing. Still getting used to the idea of compartments.

@nicolo-ribaudo nicolo-ribaudo reopened this Oct 1, 2021
@mhofman
Copy link
Member

mhofman commented Oct 1, 2021

Are we saying JSON.stringify can unbox (even across realms)

No, it would still use the Unbox operation which is checking the realm against the execution context.

but if the replacer returned a box and this is then auto-unboxed, the remaining subtree can not be passed to the replacer.

Would it make sense to perform: replacer, auto-unbox, recurse if box? It'd be similar to the way object properties are recursed, but in place instead of nested properties. Regardless, I believe that's a separate question from virtualization. As long as a toJSON is always called after auto-unbox and before a replacer, we should be safe.

@acutmore
Copy link
Collaborator

acutmore commented Oct 1, 2021

Could the virtualisation replace and install its own implementation of JSON.stringify as an option?

These are the current stringify semantics that we landed on previously: https://github.com/tc39/proposal-record-tuple/pull/241/files - which was toJSON? -> replacer? -> unbox? -> toJSON? (if not already called previously) -> unbox?

@mhofman
Copy link
Member

mhofman commented Oct 1, 2021

toJSON? -> replacer? -> unbox? -> toJSON? (if not already called previously) -> unbox?

Yeah, preventing the second toJSON would be a problem.
E.g. the following would reveal the content of the virtualized Box

const c = new Compartment();
const box = c.evaluate(`Box({})`);
JSON.stringify({
  foo: {
    toJSON: () => box
  }
});

Could the virtualisation replace and install its own implementation of JSON.stringify as an option?

That would be extremely heavy handed and hard to keep up to date.

IMO, we should say if toJSON returns a Box (and it's not subsequently substituted by the replacer) it's not included in the output. If you want to represent your object as a value, it's on you to provide that serializable value that needs no further unwrapping (the same way toJSON in a return value is not called further). Aka toJSON? -> replacer? -> unbox? (if 'toJSON' was not called previously). The replacer will be able to differentiate "box from toJSON" and "box as box" by doing its own this[key] === value if it wants.

@acutmore
Copy link
Collaborator

acutmore commented Oct 1, 2021

The other aspect is existing compartment implementations, if one compartment is given access to the original Box and passes a Box instance onto a compartment that has not been given original Box, it could still unbox it using JSON.stringify. Is this a concern? (I hope it isn’t because that means that JSON.stringify can auto unbox same realm boxes which would be very ergonomic for a large number of JS users )

@mhofman
Copy link
Member

mhofman commented Oct 1, 2021

The other aspect is existing compartment implementations

The only existing implementation (SES shim) does not pass things on the global it doesn't know about, like Box

if one compartment is given access to the original Box

That would be a powerful endowment. The start compartment is considered powerful and it has to be mindful about what it gives out to compartment it creates. Untrusted code is not supposed to run there.

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

Ugh I think I wasn't thinking clearly when I wrote the toString and toJSON traps. This doesn't work at all...

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Oct 4, 2021

So, for safety, String(box) should either return Box() (or [object Box]) or throw?

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

If we want to preserve Compartment virtualization of Box, no operation other than an explicit Box.unbox should unbox the box.

If String(box) and JSON.stringify defer to Box.prototype.toString and Box.prototype.toJSON respectively, then SES lockdown could replace those with tamed versions (returning e.g. Box() and undefined). The problem is that it may be unexpected for programs to have such a different behavior under lockdown or not (the current differences are very limited and mostly compatible, except for the override mistake).

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

The alternative is to somehow have an unboxing hook which is current compartment sensitive, which would be a significant departure from current patterns.

Last option is to give up on Box virtualization (which would permit unbox to move back to the prototype).

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

@nicolo-ribaudo, let's discuss virtualization impact here instead of #232.

One last option is to have have JSON.stringify rely on Box.prototype.toJSON for any implicit unboxing, and as @acutmore suggested, have JSON.stringify be unique per compartment. It'd basically work by wrapping the replacer with a version that unbox boxes in a compartment aware way.

It'd require replacing (or removing) Box.prototype.toJSON in a lockdown environment, which may be acceptable (depending on how much code directly calls it).

@nicolo-ribaudo
Copy link
Member

If JSON.stringify needs to be replaced by SES, we might not need to also defer to Box.prototype.toJSON?

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

I suppose. If the per-compartment JSON.stringify makes sure to never expose a Box to the original algorithm.

@erights did express concern about the semantics of auto-unboxing, and why JSON stringifying would be special. I know one concern was the bad experience developers had with BigInt, however the problem is different since those carry a simple value, and don't have semantics attached to them.

@erights
Copy link

erights commented Oct 4, 2021

This whole discussion is distressing. We need a simple theory of what kind of thing a box is supposed to be. Is it transparent or opaque? If it is opaque, then it should never be dereferenced transparently. If it is transparent, then it should always be, including across realms. Also, if it is transparent, why is it needed? Why cannot R&T just contain objects directly? Each of these, taken by themselves, have good answers. However, we must answer them so we have an overall coherent story. What is a box about? How should we understand it in terms of hard lines between what it makes possible and what it makes impossible?

My overall general sense of box is towards it being opaque, where for each box, only its associated Box constructor's original Box.deref can dereference it. If so, then that "only" rule should be absolute. If that "only" rule is not absolute, we need to revisit the entirety of the rest of the design to make it consistent with that decision. I don't see any easy answers on the non-absolute side except full transparency, but perhaps I'm missing something.

@ljharb
Copy link
Member

ljharb commented Oct 4, 2021

I'm still very unconvinced that one realm's Box should not be able to dereference another realm's Box - that's not the way internal slots work in the language, and template literals' per-realm state is an anomaly that i don't think we should repeat.

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

My understanding was the opposite, and that any new way to mangle object graphs cross-realms should not be allowed, or at least should be explicit. Also to be pedantic, you can unbox another realm's box, if it contains a primitive. And you can always unbox another realm's box, even if it contains and object, if you have access to that other realm's unbox.

@acutmore
Copy link
Collaborator

acutmore commented Oct 4, 2021

I'm still very unconvinced that one realm's Box should not be able to dereference another realm's Box - that's not the way internal slots work in the language, and template literals' per-realm state is an anomaly that i don't think we should repeat.

My understanding of the issue is that this is perhaps less about what we want and more about what we have. What has been stated is that there are existing applications that are currently using realms combined with a isObject check as a way to keep multiple object-graphs separate. If a proposal introduces new primitives (isObject:false) that provide access to values that are isObject:true then these applications are vulnerable unless they are able to update their code, which may not be possible.

Making cross-realm access to the contents of a Box opt-in then these existing applications will by default get the security mechanism they would need to remain secure.
Applications not wanting this behavior will need to opt-out by overwriting their globalThis.Box with the Box from the other realm they wish to share boxes with, doing this at the very start before Boxes are constructed. Or passing the required Box.unbox function to the other realm when it is required.

@ljharb
Copy link
Member

ljharb commented Oct 4, 2021

If the choices here are between preserving one and breaking one of the following invariants:

  1. slots and builtins all work cross-realm
  2. isObject checks can be used to keep object graphs separate

My intuition is that the first invariant is relied upon FAR, far more than the second, given how long iframes have been a thing, and this leads to my belief that we should weigh preserving the first invariant over the second.

If we're unwilling to do that, then I think there's a third option, which is that we don't provide Box at all.

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

Moved the Realm discussion to its own issue (#260), I'd like to keep the topic here on virtualization.

@ljharb
Copy link
Member

ljharb commented Oct 4, 2021

Totally fine to move it to its own issue; but I don't think the topics are really separable, if virtualization depends on the "separate object graphs" thing that conflicts with cross-realm access.

@erights
Copy link

erights commented Oct 4, 2021

FWIW I agree these are inseparable, but separate issue is fine if we keep this in mind.

@mhofman
Copy link
Member

mhofman commented Oct 4, 2021

You're right, there is a similar problem:

  • 2 ShadowRealms have distinct object graphs. No one realm can directly observe an object from another realm.
  • The object graph from Compartments look more like a tree structure. Outside of the frozen primordials (ground layer?), a child Compartment can only access the objects which were explicitly endowed, either at construction or through arguments.

However the overall object graph is still shared between compartments, and we could decide that an explicit "containsObject" predicate is sufficient to control the sharing of objects between compartments (basically assume that a box primitive carries the same authority as its content).

In my mind, the cross-realm restriction is a much stronger boundary which we can't break, unlike the compartment one.

@nicolo-ribaudo nicolo-ribaudo added the boxes All the discussions related to built-in object placeholders label Dec 17, 2021
@nicolo-ribaudo
Copy link
Member

We removed boxes from the proposal, since it was stalled because of them. I'm closing this issue, but we'll keep track of it if we'll bring them up again as a follow on proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
boxes All the discussions related to built-in object placeholders
Projects
None yet
Development

No branches or pull requests

6 participants