tc39 / proposal-array-is-template-object

TC39 proposal to identify tagged template string array objects
https://tc39.es/proposal-array-is-template-object/
MIT License
44 stars 7 forks source link

Resolve Realm-agnosticism #10

Open mikesamuel opened 5 years ago

mikesamuel commented 5 years ago

This is a separate consensus seeking issue to resolve a blocker for #2.

+@ljharb who suggested Realm-agnostic current semantics. +@erights who has concerns about the proposal in its current form and is a reviewer +@jridgewell who is the other reviewer. +@littledan who raised #3

Please correct me if I'm wrong, but IIUC, in https://github.com/tc39/proposal-array-is-template-object/issues/9#issuecomment-501697638 Mark is concerned that to preserve Proxy transparency (which impacts SES), a proxy-compatible version of Array.isTemplateObject would need a step 3 like that in Array.isArray.

7.2.2 IsArray ( argument )

The abstract operation IsArray takes one argument argument, and performs the following steps:

  1. If Type(argument) is not Object, return false.
  2. If argument is an Array exotic object, return true.
  3. If argument is a Proxy exotic object, then a. If argument.[[ProxyHandler]] is null, throw a TypeError exception. a. Let target be argument.[[ProxyTarget]]. a. Return ? IsArray(target).

The previous spec text was realm-gnostic.

https://github.com/tc39/proposal-array-is-template-object/blob/f689383dbf9061497dbc8b6c421619ec0eb6b730/spec.emu#L37-L44

Which affected this test:

https://github.com/tc39/proposal-array-is-template-object/blob/6c8e1e0e1970ad8dc0e4e44d84fb5a92cf9f60a9/test262/test/built-ins/Array/is-template-object.js#L44-L48

Is that an accurate summary?

Would people be ok with going back to the realm-gnostic version?

ljharb commented 5 years ago

I wouldn’t. You already have to virtualize everything that does brand checks in order to control cross-realm behavior; why is this any different than all the other mechanisms?

The Array.isArray behavior is currently an aberration; it’d have been nice if it applied to all internal slot lookups, but as it is, i don’t see the value in adding a second special case.

mikesamuel commented 5 years ago

@ljharb To be clear, I'm not proposing replicating the Array.isArray behavior.

I'm asking about going back to the realm gnostic behavior: https://github.com/tc39/proposal-array-is-template-object/blob/f689383dbf9061497dbc8b6c421619ec0eb6b730/spec.emu

ljharb commented 5 years ago

That doesn’t make any sense to me either - if the point of this proposal is to identify objects created by a literal template tag, why would one in another realm be any different?

Slots are cross realm. Unless all of them become same-realm (i doubt this is web compat), they should all stay cross-realm.

mikesamuel commented 5 years ago

@ljharb , @erights has some concerns about interactions with js-membranes.

I think the outcome that he wants to avoid is a realm-agnostic Array.isTemplateObject returns false for a faithful proxy of a template array from another realm.

I think the idea is that if you have two realms, R and S, connected by some channel C.

Presently, the realm-agnostic Array.isTemplateObject(op) returns false.

My understanding is that @erights would be ok with

ljharb commented 5 years ago

The way to manage that is the same way to manage everything else that brand checks - virtualize Array.isTemplateObject too.

I don’t think it’s a reasonable request to repeat the deviation that is isArray anywhere else, when there’s hundreds of other places the standard pattern (of virtualizing the brand checker) applies.

mikesamuel commented 5 years ago

@ljharb, In this scenario, realm S is using its own Array.isTemplateObject.

I don’t think it’s a reasonable request to repeat the deviation that is isArray anywhere else

AFAICT, noone is requesting that. That's why I'm asking about realm-gnostic Array.isTemplateObject.

jridgewell commented 5 years ago

There shouldn’t be a need to wrap o in a proxy, it’s a deeply frozen object.

ljharb commented 5 years ago

@mikesamuel some current axioms we have are a) internal slots are always cross-realm, b) except for Array.isArray and [[Call]], Proxies do not forward internal slots. Realm-gnostic violates the first, Proxy unwrapping violates the second.

mikesamuel commented 5 years ago

There shouldn’t be a need to wrap o in a proxy, it’s a deeply frozen object.

Deeply frozen objects may have unfrozen prototypes.

const o = {};
console.log(o.x);  // -> undefined
Object.freeze(o);
console.log(o.x);  // -> undefined
Object.getPrototypeOf(o).x = 1;
console.log(o.x);  // -> 1
mikesamuel commented 5 years ago

@erights Thanks for organizing that discussion. I hope you're feeling better. I know you still have some outstanding concerns, and you're short on time. Are there any questions I might help provide clarity on?

erights commented 5 years ago

We discussed my major concern: That the security purpose of addressing your threat model is only well served by a per-realm (in particular, per-compartment) check. However, we did not get to a secondary-but-important concern: yet another compromise of membrane transparency. We should stop adding cross-realm internal slots. Each one creates a painful difference between a root-realm boundary and a root-realm-membrane boundary.

Mostly, this non-transparency is not visible to normal use patterns, because normally builtin methods that operate on an exotic object are looked up from the exotic object. It's the difference between:

aDate.getFullYear()  // works when a sDate is across a membrane
Date.prototype.getFullYear.call(aState)  // does not

However, the difference between arrays and non-arrays is more visible, via Array.isArray, JSON.stringify and more. Thus, we made array-ness punch through proxies. If we support a realm-independent Array.isTemplateObject on Array, then we should have template-ness also punch through proxies. Unlike Date.prototype.getFullYear.call(aState), Array.isTemplateObject would be too visible and too clearly marked for normal use to do otherwise. But we also need to beware the slippery slope. We must stop adding cases to what punches through proxies. Rather, we need to stop adding new cross-realm exotic internal slots.

A reminder: If we do make the test per-realm and thus per-compartment, we should put the test on eval, i.e., eval.isTemplateObject rather than Array.isTemplateObject, since the test will be specific to an evaluation context.

ljharb commented 5 years ago

How else can i do a cross-realm brand check?

mikesamuel commented 5 years ago

@ljharb

If Array.isTemplateObject is per-realm, and it lived at Array.isTemplateObject then a cross-realm-predicate could be done approximately (breakable via intrinsic tinkering) by reaching through the prototype to get the appropriate per-realm predicate:

function isTemplateObjectCrossRealm(x) {
  // Not a source of error since all template objects are arrays.
  if (!Array.isArray(x)) { return false; }
  const proto = Object.getPrototypeOf(x);
  // Not a source of error since all template objects have a prototype that is %Array%.prototype
  if (!Array.isArray(proto)) { return false; }
  const { constructor } = proto;
  // A possible source of error due to tinkering in x's realm.  May false negative.
  if (typeof constructor !== 'function') { return false; }
  const { isTemplateObject } = constructor;
  // A possible source of error due to tinkering in x's realm.  May false negative.
  if (typeof isTemplateObject !== 'function') { return false; }
  // A possible source of error due to tinkering in x's realm.  May fail either direction.
  return !!isTemplateObject(x);
}
ljharb commented 5 years ago

That can’t be trusted because the other realm may have replaced the function - the only thing i can trust is caching a function in my own realm prior to untrusted code running.

mikesamuel commented 5 years ago

@ljharb. True. Hence the "breakable via instrinsic tinkering".

ljharb commented 5 years ago

Breakable means it’s not a brand check :-)

mikesamuel commented 5 years ago

the only thing i can trust is caching a function in my own realm prior to untrusted code running.

Quite right, a parent realm could maintain a Map from descendent Realms' Array.prototype to isTemplateObject.

mikesamuel commented 5 years ago

Edited language s/brand check/predicate/.

A predicate is sufficient for the non-security use cases.

The per-realm brand check is sufficient for known security use cases.

littledan commented 5 years ago

I don't understand the language about "yet another compromise of membrane transparency". Membrane transparency is nowhere near the default. Almost nothing using internal slots operates in a membrane-transparent way. Private fields are similarly non-transparent. The proposal is simply following the normal pattern. I'm not in favor of making a habit of piercing through Proxies in cases like this.

erights commented 5 years ago

Almost nothing using internal slots operates in a membrane-transparent way.

Almost all use of internal slots is membrane transparent. See the Date example at https://github.com/tc39/proposal-array-is-template-object/issues/10#issuecomment-524248326 above.

Private fields are similarly non-transparent.

Private fields are completely fully totally membrane transparent with no exception. If realm R1 has a class C1, from evaluating source code for class C, and realm R2 has a class C2 from evaluating the same code C, C1 and C2 already cannot access each other's private fields. If you place a full membrane between these realms, it is behaviorally identical. C1 only encounters membrane proxies for C2, and vice versa. But what they see is that they cannot access each other's private fields in exactly the same way. Neither can tell that they're talking to proxies rather than the other class.

Were internal slots accessible only per realm, and not across realms, then membrane transparency would be almost perfect across the entire language.

erights commented 5 years ago

I'm not in favor of making a habit of piercing through Proxies in cases like this.

Neither am I. I prefer not to create the problem in the first place.

littledan commented 5 years ago

I'm sorry, I used "membrane transparent" when I meant to say "proxy forwarding". I believe none of these forwards through Proxies, and all of them should work out well with membranes (including the current specification draft as is).

erights commented 5 years ago

The problem is that

aDate.getFullYear()  // is normal usage
Date.prototype.getFullYear.call(aState)  // is unusual, so breakage less painful
Array.isArray(x)  // is normal usage, so breakage would have been painful
Array.isTemplate(x)  // would be normal usage, so breakage would be painful

That last line would become true with this proposal as it stands, creating a problem that we have successfully avoided.

I recommend that we fix this by making the Array.isTemplate test per-realm, which we can do easily since the specification already has the semantic state (specification data structure) of a per-realm collection of all templates created in that realm. We need merely specify that eval.isTemplate query that data structure in its own realm. We do not need to add any more semantic state to the spec to accomplish this.

Only if we make the terrible decision to make this test cross-realm, do I advocate that it punch through proxies, as Array.isArray did. I agree this would be horrible. Please let's not create the need for this kludge.

koto commented 3 years ago

Hi! I'll take over championing this from @mikesamuel.

Things might have changed since the last update here, and I was also not part of the offline discussion about this issue, so I might be lacking some context. Perhaps some of those concerns are no longer true. What are the next best steps, given that there seems to be conflicting and mutually-exclusive requests from @ljharb and @erights ?

Having read this thread, I disagree that we should replace the Array.isTemplate with eval.isTemplate - it's very non-idiomatic, and I don't see a strong enough reason to do so. Realm-agnostic check is preferable for web-compatibility and the reasons already mentioned in the thread, and the implementation based on internal slot follows from that. I agree with @ljharb we should not follow the proxy-piercing Array.isArray hack here.

@erights, how is Array.isTemplate case in your example different from Date.prototype.getFullYear.call(aState) and why you don't consider the latter an issue? As currently described, you seem to argue that the former might become "popular", "too clearly visible and marked for normal use", while the latter is rare. Can one account for that fact and simply virtualize Array.isTemplateObject like @ljharb suggests? That seems like a simple solution to a problem that very rarely would occur in practice, whereas realm-gnosticism is very invasive to the spec.

koto commented 3 years ago

@erights, friendly ping.

koto commented 3 years ago

Moved back here for clarity, this is a continuation of the discussion with @erights in https://github.com/tc39/proposal-array-is-template-object/issues/15#issuecomment-764853568.

The terms "security" and "security boundary" are unclear umbrella terms that mean different things to different people. See A Taxonomy of Security Issues for understanding language-based security and modularity and please explain in more precise terms what you mean. I suspect this remains the heart of our disagreement.

In my understanding (apologies If I misunderstood it!) the essay proposes a taxonomy that seems to be advocating for creating finer-than-process-level boundaries inside the language, and claims that they are secure - as in; they preserve integrity across boundaries (as a sidenote, I don't think that claim is sufficiently evidenced in the essay, but I'm definitely missing a larger context).

Membranes based on Proxies can surely be used to provide some level of separation between different parts of code; both sides of the membrane could also be running in different Realms. Such a setup could be called a security boundary within some threat model, but it doesn't follow that Realms are security boundaries themselves - and Javascript already does not make it easy to use them as such. Even with identity discontinuity there are many intentional "leaks" across Realms (e.g. internal slots).

If I understand correctly you oppose adding an algorithm that relies on the internal slot value (and as such is cross-realm). Would you agree that we should also change other algorithms not to use internal slots? For example:

// anotherRealm is e.g. a sameOriginIframe.contentWindow

JSON.stringify({"foo": 1, "bar": 1}, [new anotherRealm.String("foo")], new anotherRealm.Number(4))
ArrayBuffer.isView(new anotherRealm.Int32Array());
Promise.resolve(anotherRealm.Promise.resolve(4))

All of the above functions will read from internal slots, and, if I understand your objection, without proxy-piercing, they are not "safe" (please explain safety in this context, I feel we still are not on the same page here).

Note that I avoided using vectors based on prototype like in https://github.com/tc39/proposal-array-is-template-object/issues/10#issuecomment-527172900 as you consider these unusual. Calling functions on objects from a different Realm is instead quite common (imagine the objects are not created inline like above, but taken from their realm elsewhere). What makes Array.isTemplateObject worse than the established Promise resolution mechanism?

I am not. [OK with the limitation that eval in any reachable Realm weakens the brand check as the template objects can be created from strings there].

Can you expand on that? Can't the eval-capable Realm simply redefine Array.isTemplateObject in the checking Realm instead or otherwise confuse this check? In any case, many existing mechanisms may be subverted from dynamic code evaluation (and we're tackling that too separately!) and I don't think Array.isTemplateObject is very special in that regard. With eval() in any Realm you can't really trust your prototype chain, Function.apply etc.

I also don't see a way around it. For a truly robust-against-eval (even only local eval!) mechanism the Array.isTemplateObject would have to throw under eval presence (I guess detected via a host hook), and that seems undesirable to me. Array.isTemplateObject is "just" a brand check, with all regular limitations.

That said, it is still possible to create a robust check - just assert that eval is blocked in all participating Realms.

The argument I find the most convincing for cross-realm check as a primitive is that it's easy to go from it to the same-realm one, whereas the opposite is very difficult and requires tracing Realms as they get created to capture their prototypes https://github.com/tc39/proposal-array-is-template-object/issues/10#issuecomment-524337885. Would you agree?

erights commented 3 years ago
JSON.stringify({"foo": 1, "bar": 1}, [new anotherRealm.String("foo")], new anotherRealm.Number(4))

Is this the example you meant? I just tried it in Chrome with and without anotherRealm. and the output looks the same. See attached screenshot.

cross-realm-json
erights commented 3 years ago
cross-realm-isView
erights commented 3 years ago

What happens with these across transparent membranes? I have not yet tried that.

erights commented 3 years ago

Assuming the JSON example does not work across membranes, the problem is the use of the object wrappers for primitive types. For strict code, this object wrapping never happens implicitly. There is no reason for normal code to ever use object wrappers explicitly. At least I have never found a reason. In their absence, the first example doesn't arise.

The main internal state test that I'm aware of that JSON.stringify does is to test arrayness, the equivalent of Array.isArray. By design this does punch through proxies and so is transparent across membranes.

erights commented 3 years ago

I frankly have not thought about ArrayBuffer.isView before. I need to examine this. Until I do, I must acknowledge that this may be a genuine problem.

erights commented 3 years ago

Promise.resolve is a known problem. It is less transparent than I'd like but more practically transparent than you might expect. The problem is that Promise.resolve does not recognize a proxy for a promise as a promise, and so coerces it rather than returning it uncoerced. The reason it is still practically transparent enough for most purposes is Promise.resolve still recognized the proxy as a thenable and so still assimilates it the way it does other thenables.

Thenable assimilation was designed in the first place when we were introducing ES6 promises into a JS world already populated by a zillion other promise libraries. Thenable assimilation allowed each to treat the promises from the other as promise-like enough that a lot of code worked without needing to be aware that those other alleged promises were not "really" promises. That observation applies equally to this case.

littledan commented 3 years ago

@erights I feel that, as we evolve and grow the JavaScript standard library, it will be too limiting to refrain from using internal slots of function arguments, and JSON.stringify and ArrayBuffer.isView. (To clarify about JSON.stringify: it also checks whether primitive wrappers are really primitive wrappers.) Membrane systems will have to unpack parameters if they want to virtualize a JavaScript standard library--it's the same for these functions as the many DOM methods which read internal slots.

erights commented 3 years ago

it will be too limiting to refrain from using internal slots of function arguments

Class private fields, which were initially intended to be internal-slot-like, instead adopted a weakmap-like semantics. Along the same lines, @domenic at one point proposed having new internal slots not be cross-realm accessible. Both semantics would avoid these problems without impeding intra-realm expressivity.

Attn @fudco

erights commented 3 years ago

In any case, many existing mechanisms may be subverted from dynamic code evaluation

If one eval is not able to magically brand things such that another eval recognizes them as branded, then from the pov of the second eval how is the first eval different than a meta-interpreter written without using an eval primitive? An eval restricted environment is clearly not in a position to ban evaluation by meta-interpreters. An eval restricted environment is therefore equally protected from other evals by not giving them more power from its pov than they would have if they were meta-interpreters. An unprivileged eval-free meta-interpreter, it would not be able to brand a string computed at runtime as a template that another eval would recognize. Thus another eval should not recognize one so branded by a builtin but foreign eval.

koto commented 3 years ago

Thanks for your updates, they are very helpful to me. Your point earlier in the thread is to "stop adding new cross-realm exotic internal slots", I was trying to show that functions depending on them (and, as such, working with cross-realm objects - "realm-transparent" if you will) are common enough already. I was purposefully ignoring the membranes to simplify the argument.

Perhaps there is some carveout in Proxy that makes some of this cases less problematic. For example, you mentioned that "builtin methods that operate on an exotic object are looked up from the exotic object" helping one preserve membrane transparency -- there's also Array.isArray hack, for example.

That said, I feel my examples (and I'm sure others can be found, this was just me skimming through some internal slots usages) demonstrate that internal slots for various checks are a standard, and not a fringe use. And of course some can be argued against, but I think the point holds that designing for membrane transparency -- in the current language -- is not a norm, unless you squint and ignore several cases, in a way that feels subjective. A practical ban on internal slots usage (I'm not sure if that's what you propose, but it feels like a natural consequence) feels very limiting to me.

erights commented 3 years ago

@littledan puts it well. For every internal slot we also define builtin methods that recognize the internal slot. If those builtin methods only recognize it on their this argument, then it falls under the "practically transparent" caveat. Otherwise, we need to resort to one of these other semantics that make the internal state, represented somehow, not be accessible cross-realm in ways that they would not be across a membrane.

Of the three examples you found, two are not terrible. I remain worried about isView. We should continue to gather examples and categorize them to see what the existing situation really is, and how much or little practical transparency we currently have.

koto commented 3 years ago

If one eval is not able to magically brand things such that another eval recognizes them as branded, then from the pov of the second eval how is the first eval different than a meta-interpreter written without using an eval primitive? An eval restricted environment is clearly not in a position to ban evaluation by meta-interpreters. An eval restricted environment is therefore equally protected from other evals by not giving them more power from its pov than they would have if they were meta-interpreters. An unprivileged eval-free meta-interpreter, it would not be able to brand a string computed at runtime as a template that another eval would recognize. Thus another eval should not recognize one so branded by a builtin but foreign eval.

I might be not following fully, there's a lot to unpack here, but you seem to argue that if eval would be incapable of branding template objects for another Realm (or e.g. eval within that second Realm), then the target Realm can ignore the evalness of the first Realm for this brand check? (I assume evals are in different Realms, maybe that's not what you mean). That is correct, but it's not what is proposed. eval can create the brand, and the brands are recognized cross-realm. I might be totally missing the point, sorry, could you clarify with a code example?

erights commented 3 years ago

First, yes, I assume different realms have different evals. Further, I assume that different compartments have different evals.

If the brand was represented as a per-compartment weakset, and only checked in one's own weakset, then something that was created as a template by one eval would not be recognized as a template by another eval. That's the solution that @mikesamuel discussed and agreed on. Since you expect to turn off all evals in the scenario you have in mind, how does this narrower semantics cause you any problems?

Imagine two primitive-eval-free meta-interpreters: evalA and evalB, that do not know about each other. Each tries to implement the language including your proposal, as completely as possible while remaining safe in their own terms. Since they don't know about each other, let's start with the assumption that they each create their own primordials, emulating separation similar to that between realms. To code that has access to both:

var t1 = evalA('(t => t)`foo`');
evalA('t => Array.isTemplate(t)')(t1); // true
evalB('t => Array.isTemplate(t)')(t1); // false

This works with disjoint primordials since they each have their own Array constructor. At this point my argument is that the primitive evals of different Realms should have the same interaction --- they should not recognize each other's alleged templates as legitimate templates.

Within a frozen Realm after a SES lockdown(), we could equally well construct two eval-like but primitive-eval-free meta-interpreters in two separate compartments. Since all the shared primordials are frozen, the two meta-interpreters remain fully isolated.

The same principles apply except the code above does not. The reason is that they both share the same Array constructor. However, each compartment has its own eval, Function, and globalThis. If the isTemplate test were hung off of any of these then it would be per-compartment. Since the difference between evals is the focus issue, I suggest that's the most natural place to put it

var t1 = evalA('(t => t)`foo`');
evalA('t => eval.isTemplate(t)')(t1); // true
evalB('t => eval.isTemplate(t)')(t1); // false

but either of the following also works

var t1 = evalA('(t => t)`foo`');
evalA('t => isTemplate(t)')(t1); // true
evalB('t => isTemplate(t)')(t1); // false

as well as

var t1 = evalA('(t => t)`foo`');
evalA('t => Function.isTemplate(t)')(t1); // true
evalB('t => Function.isTemplate(t)')(t1); // false
erights commented 3 years ago

and the brands are recognized cross-realm

My point is exactly that this is a bug, not a feature.

koto commented 3 years ago

and the brands are recognized cross-realm

My point is exactly that this is a bug, not a feature.

Do I understand correctly that your point about eval is only relevant if the brand is not carried across Realm? That seems to be what the examples assume. If so, then I think I understand how this may introduce complexities you described, but again -- this is not what I propose. I'm trying to understand whether, were you convinced about cross-realmness, there is still another remaining issue. Thanks again for explaining your point!

erights commented 3 years ago

If we make it recognized cross-realm, then it would either

Here's a proposal that I can't say I love, but would fall into the second category:

Move your isTemplateObject onto Array.prototype as a no-argument method that tests its this. Then, the "normal" test would be

arrayWhichMightBeTemplate.isTemplateObject();

which would work fine across membranes. Code that wants to work security-equivalent to your current proposal could instead do

Array.prototype.isTemplateObject.call(arrayWhichMightBeTemplate);

The key is which we expect "normal" code to engage in. If the second remains as obscure as other similar-looking cases, then it does fall into that loophole.

My preference would still be to have template-ness not be recognized across compartments or realms. But I admit the prototype variant above does not hit any show stoppers.

koto commented 3 years ago

I must say I don't hate it :) To double check, eval would not be an issue? The solution solves for "practical membrane transparency", does it also satisfy your concerns w.r.t. eval?

ljharb commented 3 years ago

I'm fine with that as an alternative proposal, but we have a number of brand-checking non-proxy-piercing non-prototype methods already - not just Promise.resolve and ArrayBuffer.isView, but also the other 3 Promise combinators, and all (13?) of the Atomics methods (which read TypedArray internal slots).

In other words, it seems like the cat is already out of the bag on this one.

erights commented 3 years ago

I must say I don't hate it :) To double check, eval would not be an issue? The solution solves for "practical membrane transparency", does it also satisfy your concerns w.r.t. eval?

The

arrayWhichMightBeTemplate.isTemplateObject();

alternative solves the practical membrane transparency issue but not the eval issue.

The

eval.isTemplateObject(allegedTemplate);

solves both. The eval issue is the bigger deal.

erights commented 3 years ago

I'm fine with that as an alternative proposal, but we have a number of brand-checking non-proxy-piercing non-prototype methods already - not just Promise.resolve and ArrayBuffer.isView, but also the other 3 Promise combinators, and all (13?) of the Atomics methods (which read TypedArray internal slots).

In other words, it seems like the cat is already out of the bag on this one.

If these are all the existing cases, then I draw the opposite conclusion. All the promise methods will assimilate thenables, achieving a significant degree of practical transparency.

The Atomics are low level direct memory manipulation that we should never expect to work across membranes. They will remain obscure to normal users.

It sounds like we're down to one problem case, ArrayBuffer.isView. I am more confident than before that we have enough practical membrane transparency to be worth preserving.

koto commented 3 years ago

Imagine two primitive-eval-free meta-interpreters: evalA and evalB, that do not know about each other. Each tries to implement the language including your proposal, as completely as possible while remaining safe in their own terms.

[...] my argument is that the primitive evals of different Realms should have the same interaction --- they should not recognize each other's alleged templates as legitimate templates.

I think the underlying issue is that complete meta-interpreters must be created with the respect of internal slot behavior in ES. Your proposed fully isolated meta-interpreter would already face the same incompatibilities in other parts of the language, since internal slots are purposefully accessible across realms - and, for example String instances would not behave as they are defined. Nor do I see how evalA would recognize evalBs Elements in exactly the same way DOM does without introducing custom logic around them. I fully agree that an 'almost complete' (practical?) emulation is much easier than a full one.

If an interpreter design assumes full Realm isolation, then it's not implementing ECMAScript, at least not when multiple Realms are at play. I understand how it creates a complexity for creating VMs, but I believe it's not the first occurrence of this pattern, it's also quite unlikely it would be a last one. A robust ES emulation either has to acknowledge how the language is defined fully, or make adjustments that it deems match its purpose (e.g. disallowing using certain features, or emulating them in a limited way). To my understanding any slot usage in the spec creates the same issue for fully isolated environments that, in the end, exchange objects and needs handling somehow, at the extreme end just acknowledging that a feature is rare and not supported, at the other creating a working virtualization -- which, for isTemplateObject doesn't actually seem to be hard. Would you agree with the above?

erights commented 3 years ago

I think you're missing the point of the meta-interpreter exercise. Whether String wrapper objects are recognized cross-realm or across membranes is an annoyance issue, and it the two answers are inconsistent is a transparency issue. But beyond transparency, the String wrapper issue is not a security concern one way or the other.

Your motivation for introducing isTemplateObject is to achieve a security goal. The meta-interpreter gedanken experiment shows that you achieve a stronger form of your own goal by making it eval-relative. Put another way, the meta-interpreter exercise demonstrates that the security goal you achieve by not making it eval relative is much weaker than you think it is.

You would not want to allow foreign unprivileged meta-interpreters to be able to label something as a template in a universal manner. You should similarly not want to allow foreign evals to do so. If you do allow this, the isTemplateObject test becomes less meaningful. What security goal do you think you're achieving when the object graph has multiple co-existing evals evaluating code from different sources? If you make the test eval-relative, it continues to serve your existing purpose and extends your own purpose to give a meaningful answer in this more challenging --- and increasingly common --- scenario.

Finally, the spec already has the semantic state needed to do this test in this more robust way --- the template cache. It is already realm-relative. It could be made eval-relative with no incompat from the status quo. An eval-relative test could just use this existing state. No need to introduce a new internal slot which is redundant with semantic state we already have.

koto commented 3 years ago

Thanks, that clarifies your position to me much better.

Your motivation for introducing isTemplateObject is to achieve a security goal. The meta-interpreter gedanken experiment shows that you achieve a stronger form of your own goal by making it eval-relative. Put another way, the meta-interpreter exercise demonstrates that the security goal you achieve by not making it eval relative is much weaker than you think it is.

Perhaps it's a matter of the threat model. In the one I'm coming from the weaker form is "good enough", as it already aligns with the environment web applications operate in, hence the pushback. The native boundaries there are not Realms, or compartments, they are set at a different level - process boundaries, browsing context groups, origins etc. I agree that Realm-local isTemplateObject provides stronger guarantees and it's a good coincidence that there is a way of reusing the template cache. It's not terribly convenient should one want to make a cross-realm check, but perhaps that's for the better. @jridgewell and @ljharb, what do you think - does that align well with where ES is going?

I'm not sold on making it eval-relative, especially putting the brand check function under eval or Function seems non-idiomatic. You mention that, even if the brand-check was Realm-local (using the template cache), putting it under Array is a mistake. Citing your example:

Within a frozen Realm after a SES lockdown(), we could equally well construct two eval-like but primitive-eval-free meta-interpreters in two separate compartments. Since all the shared primordials are frozen, the two meta-interpreters remain fully isolated.

Is it true? Frozenness doesn't imply that the state is not shared. For example, the per-realm Template Map is shared. I'm not sure if the proposal demonstrates more than the SES lockdown() has some caveats to consider, rather than there is something flawed in putting the same-realm brand-check under Array (or the prototype function).

Thanks for spending the time on this, I think we are converging on something really useful.

ljharb commented 3 years ago

I’m not sure i see a trend to respond to - cross-realm checks aren’t going away, and remain important. Making it eval-relative does seem workable, however, since then the question it’s answering is “is this a template object that came from this eval function?”, which means as long as i can access that eval, i can check from any realm.

It certainly has always seemed weird to me to have this check on Array since it’s really not generic array functionality (we don’t have Array.isMatchObject, for example). However, if it is on Array or Function, or even Array.prototype, i think same-realm is a mistake.