Return value instead of the getter function for rewritten intrinsics

[iFwu]

Under some environments like WeChat Mini Program, built-in intrinsics are rewritten to prevent further modification. And the over-written code use getter/setter to hook property access and setting.
For example, when calling GetIntrinsic('%Reflect.apply%') it will return the override getter function instead of the apply function itself. That will cause many TypeError during the initialization of the es-extract module.

One possible fix is to list all native properties that use the getter/setter, e.g. Map.prototype.size. Only when getting such properties, return the getter. Otherwise, GetIntrinsic always return the value return from get().

[ljharb]

This is also something SES does, and it presents a problem, because an environment that starts out with specified data properties being accessor properties is not a spec-compliant javascript environment.

I agree that explicitly listing either every intrinsic that's a data property, or every one that's an accessor, would address this problem, but we'd still be accounting for a broken environment.

SES handles this by only turning things into getters when strictly necessary - which is not the case for Reflect.apply, for example. Perhaps WeChat miniprogram could use SES rather than incorrectly reinventing a solution for the problem of safely running code?

[ljharb]

cc @erights @kriskowal for visibility

[kriskowal]

Does the value returned by the getter vary? I don’t think there’s a problem with GetIntrinsic returning the value of the intrinsic’s property descriptor or the value returned by the intrinsic’s property getter if those values would be the same. I believe from the SES perspective, if we had a GetIntrinsic intrinsic to work with, we would expect it to return the actual intrinsic, not the one we patched onto another intrinsic’s properties. To preserve isolation, we either wouldn’t vend GetIntrinsic to guest code or we would “tame” it such that it can only reveal our “tamed” shared intrinsics.

cc @erights for review.

[mhofman]

I believe the taming through accessors is currently only necessary to handle the override mistake, and in a perfect world, we could freeze intrinsics without triggering it.

That said, any modifications to the shape of the intrinsics should be reflected onto GetIntrinsics by the library doing the modifications by also replacing GetIntrinsics itself. It is however unclear what such replacement should do:

• either it can reflect reality and expose those accessors only, resulting in missing expected values, which would cause the issue above
• or it can lie and pretend the intrinsics are still pristine

My guess is that a 3rd option would actually work in most cases: pretend that they all exist. E.g. the replacement would have answers for all of %Object.prototype.toString%, %get Object.prototype.toString% and %set Object.prototype.toString%.

[ljharb]

If it’s going to lie, it should patch gOPD and gOPDs on Object/Reflect too, so that they can’t discover the lie.

[erights]

Yes. The thing about shimming is that the resulting environment ideally does not differ in any observable way from the environment being emulated.

[ljharb]

If that shimming was done successfully and thoroughly, then this library would treat it like a data property, no?

[erights]

As stated above, the accessors installed by the SES-shim
https://github.com/endojs/endo/blob/8e077abc65a644225b70b1d328cde11a44364b0e/packages/ses/src/enable-property-overrides.js#L91
emulate how a frozen data property would work if tc39 had not made the override mistake. It is an imperfect emulation because we do not attempt to hide that these are now accessor properties rather than data properties. But the value returned by the getter is as stable. Thus, under the SES-shim, GetIntrinsics for these should return the same stable value that the getter returns. There is no reason for GetIntrinsics to care whether the SES-shim presents this stable value as a data property or an accessor property.

Btw, for each of these accessors, the getter installed by the SES-shim also has an originalValue frozen data property whose value is the same as that returned by calling the getter. This enables a transitive reflective property walk, such as performed by harden, to walk into these values as if these were simple data values without invoking the getter.

Currently, at https://github.com/endojs/endo/blob/master/packages/ses/src/enablements.js , the SES-shim provides three levels of compensation for the override mistake: 'min', 'moderate', and 'severe'. I consistently say "the SES-shim" above rather than "SES" or "Hardened JS" because it is not clear what the best way is for the SES proposal to specify these. Ideally, if we can get tc39 to agree, we'd specify that all hardened properties, or at least all primordial hardened properties, are frozen data properties not subject to the override mistake. Then we would not need levels of partial compensation. But we should keep this fight separate.

[ljharb]

Something else to note is that eventually the getIntrinsic proposal is highly likely to differentiate a data, get, and set property, at which point this package would follow suit - in other words, if gOPD betrays that it’s an accessor, then it will only be accessible that way.

Thus, i think that the only solution here is that whenever SES converts a data property to an accessor, it makes gOPD and friends lie and pretend it’s a data property.