endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
770 stars 68 forks source link

Propose to tc39 without override mistake #105

Open erights opened 4 years ago

erights commented 4 years ago

We should propose harden to tc39, together with whatever other primitive hardening/freezing/snapshotting primitives we need to cover our needs, including those of tc53 and Moddable.

Should we propose that properties made non-writable,non-configurable by harden also act as if the override mistake were absent? As we've seen, shimming this everywhere with accessors is expensive. But done via a standard mechanism, the platform could fix this cheaply.

Note: There was already an attempt to just fix the override mistake in the language itself. That failed under testing --- a little old code was incompatible with fixing this, but enough code that no one was willing to ratify it. The best we can get is probably that new hardening/freezing/snapshotting mechanisms fix this. Since these mechanisms would be new, the fix would effectively be opt-in and not break old code.

erights commented 4 years ago

Attn @phoddie @bmeck @jfparadis @dckc

jfparadis commented 4 years ago

Decoupling harden and fixing the override mistake make sense to me. Even in code, those steps are completely independent and are executed in sequence. The harden shim just need to become conditional to avoid overriding a native implementation.

mhofman commented 2 years ago

One idea might be to add an option to freeze (and maybe seal) to request the integrity level be cached as a slot (e.g [[CachedIntegrity]] on the object. Then we could get 2 benefits:

Object.freeze(Object.prototype, { cacheIntegrity: true });
const obj = {};
obj.toString = () => '[Overridden Object]'; // succeeds
const proxy = new Proxy({ foo: 42 }, {
  ownKeys(...args) {
    console.log('ownKeys triggered');
    return Reflect.ownKeys(...args);
  },
  isExtensible(...args) {
    console.log('isExtensible triggered');
    return Reflect.isExtensible(...args);
  },
  preventExtensions(...args) {
    console.log('preventExtensions triggered');
    return Reflect.preventExtensions(...args);
  },
});
Object.seal(proxy, { cacheIntegrity: true }); // prints 'preventExtensions triggered'
assert(Object.isSealed(proxy)); // prints nothing
assert(!Object.isFrozen(proxy)); // prints 'ownKeys triggered'
Object.freeze(proxy, { cacheIntegrity: true }); // prints 'ownKeys triggered'
assert(Object.isFrozen(proxy)); // prints nothing
mhofman commented 2 years ago

If we propose a Object.create option to deal with the return override + private fields issue, then options for sealing/freezing and caching the integrity level at creation might also make sense:

The create options bag could include an option to create born frozen/sealed objects:

const frozenObject = Object.create(null, { foo: { value: 42, enumerable: true } }, { sealed: true });
assert.throws(() => { frozenObject.bar = 'baz'; });
assert(Object.isFrozen(frozenObject));

The use cases for caching the integrity level creation are less clear, but it would look like:

function Foo () {
  this.a = 42
}
Foo.prototype = Object.create(
  Object.prototype,
  {
    toString: {
      value() { return `[Foo ${this.a}]`; }
    },
  },
  {
    sealed: true,
    cachedIntegrity: true,
  },
);
kriskowal commented 6 months ago

When we were considering this change in 2019, I did not yet appreciate that a harden that transitively freezes both properties and prototypes interferes with lockdown if it is called on almost any object. So, for harden to be more generally useful, useful before lockdown, it would probably need to be reframed as a transitive freeze over properties only.