search

tc39 / proposal-shadowrealm

ECMAScript Proposal, specs, and reference implementation for Realms
https://tc39.es/proposal-shadowrealm/
1.43k stars 67 forks source link

Host guarantees on boundary piercing #324

Closed mhofman closed 8 months ago

mhofman commented 3 years ago

A motivating use case for the callable boundary is to be able to reason about the object graph isolation.

We've focused previous invariants discussion on requiring the configurability of all globals in a new ShadowRealm so that potentially problematic host extensions could be removed.

However there are use cases that do not require the sanitation of the host environment (e.g. test runners).

Currently nothing prevents a host from exposing the object graph of another realm (likely the incubator one) through the new realm's global object. Should there be an invariant on hosts that they cannot expose a cross realm object through extensions they provide, either directly (e.g. an object or it's prototype chain) or transitively through an object involved in any chain of function calls?

Jamesernator commented 3 years ago

From what I've seen, the restrictions on hosts adding things to a ShadowRealm global are likely to be only the restriction that the properties must be configurable.

The reasoning is that SES will be containing a whitelist of all safe APIs and using .lockdown() will remove absolutely everything that is not on that whitelist. This means that hosts can expose APIs of basically any power they want on new ShadowRealms without breaking SES, as SES will simply identify such APIs as not being on the whitelist and remove them.

In this regard I don't see harm in allowing hosts to expose objects from other Realms, but it's likely a lot implementations won't expose any, particularly given that one of the motivating reasons for the current design is that it should be easier for engines to optimize JS code if they don't need to worry about objects from other Realms.

mhofman commented 2 years ago

Well the mirror of what I had raised back in September seem to be happening now in the HTML integration: Error and Promise instances from the ShadowRealm being exposed to the incubator realm. See https://github.com/whatwg/html/pull/5339#discussion_r826061561

Jamesernator commented 2 years ago

Well the mirror of what I had raised back in September seem to be happening now in the HTML integration: Error and Promise instances from the ShadowRealm being exposed to the incubator realm. See whatwg/html#5339 (comment)

Do we think hosts should be allowed to configure globalThis within ShadowRealm as long as the prototype is configurable? Because in that case the simplest solution would of course just to allow HTML to define globalThis and inheriting EventTarget as it does for every other global.

SES would just have one additional thing to do which is Object.setPrototypeOf(globalThis, Object.prototype), and because the EventTarget global would be deleted by SES anyway, isolated code would never be able to discover globalThis had another prototype to begin with.

mhofman commented 2 years ago

Maybe allowing the host to decide the prototype with the requirement that the prototype stays mutable would be an option. What would happen if the global was no longer an EventTarget and the HTML spec tried to dispatch an event on it? I think it'd be better to move the error reporting logic to a different object with its own property on the globalThis. That object can be an EventTarget and have its prototype set immutable.

Let's keep this issue focused on the requirements to impose on the host and discuss the options for the HTML integration in https://github.com/whatwg/html/pull/5339

Jack-Works commented 2 years ago

What would happen if the global was no longer an EventTarget and the HTML spec tried to dispatch an event on it? I think it'd be better to move the error reporting logic to a different object with its own property on the globalThis. That object can be an EventTarget and have its prototype set immutable.

It will have the internal slot of EventTarget, but you cannot access them via JavaScript unless you have access to EventTarget.prototype.*

mhofman commented 2 years ago

As @caridy points out in https://github.com/tc39/proposal-symbols-as-weakmap-keys/issues/27#issuecomment-1145135262, one reason for mandating host constraints on cross-realm references is that shims that want to work cross-realm could with some pain recursively wrap the ShadowRealm constructor to propagate the shim, but the shimming would be imperfect if user code could escape to another realm through host APIs like iframe.

I think the following constraints are needed:

caridy commented 2 years ago

hmm, I think I'm missing something here @mhofman. I thought those were implicit constraints imposed by the callable boundary. Can you elaborate more on how the host can bypass this?

mhofman commented 2 years ago

The host is free to specify / add any function to the global object of any realm, which can have any behavior not precluded by the 262 spec. For example, the host could decide to expose promises which were created in a ShadowRealm through the incubator realm's window unhandledrejection event. Or hosts could add an API on the global of a ShadowRealm which somehow (on purpose or not) leaks an object from another realm.

Inside ShadowRealm, these APIs would be deniable as all properties of the global must be configurable. However for the other scenario, nothing prevents the host from exposing objects from a ShadowRealm to the incubator realm.

mhofman commented 2 years ago

To give a plain example. Nothing technically prevents the host from implementing a %get ShadowRealm.prototype.globalThis% that would directly expose the realm's global to the parent.

caridy commented 2 years ago

Ok, it seems that you're asking for a normative note, is that correct? I'm fine with that. Implementers have a strong desire to prevent that from happening, so they will be fine with that too.

mhofman commented 2 years ago

Correct, I'd like these constraints to be normative. I also believe there is no desires from embedders to break the spirit of the callable boundary, I just would like it to be "enforceable". I will probably bring it up as a discussion point during the "Should rejected promises cause an event to be fired in the principal realm?" topic as that is relevant to that issue.

I'm just not sure how to formulate the constraint since there isn't really a spec notion of "objects from a certain realm". Maybe something along the lines of:

"If an execution in a shadow realm R1 oblivious of host APIs can observe the identity of an object O1, a host API Must Not allow an execution in any other realm than R1 to also observe the identity of O1. Similarly if an execution in a realm R2 can observe the identity of an object O2, a host API Must Not allow execution in any other realm than R2 that is a Shadow realm to also observe the identity of O2."

caridy commented 2 years ago

@mhofman that seems too generic, remember that there are other scenarios where other realms are reachable (iframe). Making it more specific, and high level might be possible.

Jamesernator commented 2 years ago

Making it more specific, and high level might be possible.

One way could be to add a [[Realm]] slot to every object, and add extra invariants to essential internal methods.

i.e. :

(With appropriate loosening for legacy realms).

This covers almost every possible way to observe an object, I think the only other way to observe a value is through a globally defined variable on Realm.[[GlobalEnv]], so such invariants could be specified there as well.

mhofman commented 2 years ago

I believe the wording does allow legacy realms to freely reach into each other. Which part do you think prevents that?

It basically translates as:

mhofman commented 1 year ago

@caridy do you have any further feedback on the text I provided in https://github.com/tc39/proposal-shadowrealm/issues/324#issuecomment-1145595831? As I said above I believe it only applies to situations where a ShadowRealm is involved.

mhofman commented 1 year ago

As the vm2 security incident shows, we really need to put constraint on host APIs so that if they break these invariants, it would be clear it's an implementation bug.

bathos commented 1 year ago

One way could be to add a [[Realm]] slot to every object [...]

@Jamesernator in web platform contexts, allegedly every object already has an “associated” realm — but it’s mostly not specified what that means (because of the absence of ES hooks for it?):

Although at the time of this writing the ECMAScript specification does not reflect this, every ECMAScript object must have an associated realm. The mechanisms for associating objects with realms are, for now, underspecified.

I know ES isn’t specific to web platform hosts, but I figured there’s a chance that could be pretty relevant*. Defining [[Realm]] for everything/nearly everything might solve that issue — or it might complicate it if there really are browsers already implementing something similar but with unspecified semantics.

Of course it's also possible that the quoted text is mistaken/outdated. Not 100% sure, but it seemed like no references to “associated realm” would actually be reachable with arbitrary uncallable objects. (@domenic it seemed like you’d be the most likely person on earth to know if that paragraph is legit?)

* pun intended

caridy commented 1 year ago

do you have any further feedback on the text I provided in #324 (comment)?

@mhofman I do like https://github.com/tc39/proposal-shadowrealm/issues/324#issuecomment-1146447529 a lot more, it is a lot more clear what the intent is. Can you send a PR on this repo?

mhofman commented 1 year ago

The problem is, that comment is a layman's translation. There is no definition of reachability in the spec, nor what a legacy realm is. I'd prefer the former as spec text (which is conceptually similar to the definition of liveness), maybe with an editor's note containing the latter as clarification?