endojs / endo

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

fix(static-module-record): Bind `this` and `arguments` more correctly #1775

Closed kriskowal closed 1 year ago

kriskowal commented 1 year ago

Description

Currently, the this value in module scope is globalThis. This is a gap in fidelity, since it should be undefined, but is not a leak. This change alters the signature of the module functor generated by StaticModuleRecord such that this is bound to undefined with the existing instantiation code in ses. This also ensures that the undeniable arguments lexical name reveals nothing about the evaluator or module calling convention to the module.

This will break any program that depends on this being bound to globalThis. Since all such programs were already invalid and would not work in other environments like Node.js, I am choosing to treat this as a non-breaking-change, even though it may require bug-fixes elsewhere to reconcile versions.

Security Considerations

This change does not add or remove any capabilities from the scope of modules, just requires that globalThis to be spelled out in full.

Scaling Considerations

This should not affect scale.

Documentation Considerations

This makes behavior more consistent with expectations documented for ESM generally.

Testing Considerations

I’ve included a ses test that was breaking before the change to static-module-record.

Upgrade Considerations

This change will alter the hash of the Agoric kernel modules and will get picked up when bundles get regenerated. Some contracts may have bugs where this is assumed to be globalThis.