Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How does a vetted shim extend shared global variables? #318

Closed
erights opened this issue May 17, 2020 · 7 comments
Closed

How does a vetted shim extend shared global variables? #318

erights opened this issue May 17, 2020 · 7 comments
Assignees
Labels
permits Issues pertaining to SES’s permits for properties of shared intrinsics

Comments

@erights
Copy link
Contributor

erights commented May 17, 2020

Every time a new compartment is created, its new global object is initialized by default with a standard set of global variable bindings hard coded in the ses whitelists. However, sometimes to purpose of a vetted shim is to add a new "standard" global that is not standard yet, that should be implicitly propagated to new globals just as Array is. It is not clear how a vetted shim should express this. For legacy vetted shims, it is even less clear how or whether we should automatically infer this.

Correlated with this, host independent vetted shims need not run in the dangerous all powerful start compartment. But they still need to run before lockdown(). Perhaps we support a pattern with user-level libraries, where a new default-powerless compartment is made to run the vetted shims in, and the globals those shims leave behind gets added to the implicitly propagated shared globals. If so, these new globals must also be included in what lockdown() locks down.

@erights erights changed the title How does vetted shim extend shared global variables? How does a vetted shim extend shared global variables? May 17, 2020
@erights
Copy link
Contributor Author

erights commented May 17, 2020

It looks like tameGlobalDateObject, as called by lockdown(), will replace the global variable Date with the tamed Date constructor it creates. It looks like the Compartment shim will then correctly populate new global objects with the tame Date. However, the consequences of tameGlobalDateObject should not replace the Date on the global of the start compartment.

Similarly, tameGlobalMathObject should not modify Math in place to tame it. Rather, it should leave the original full powerful one on the global of the start compartment and arrange for all other compartments, by default, to get the tames one.

I'm adding these here because the mechanisms this taming code will use to express this probably should be related to the mechanism by which we add shimmed global to the list of new shared globals.

@erights
Copy link
Contributor Author

erights commented Feb 26, 2021

We still need to confront this design issue. @kriskowal @dckc ok if I assign the three of us?

@kriskowal
Copy link
Member

I would like to investigate this plan:

lockdown remains as-is. We add a repair function that lockdown will call if it hasn’t already been called. We introduce some mechanism for modifying the SES allow-list between repair and lockdown, so an application has an interval in which it can run shims and bless whatever properties it wants to stick afterward.

@erights
Copy link
Contributor Author

erights commented Mar 17, 2021

I don't understand the role of this repair function. Is it part of SES? Is this related to the repairIntrinsics function at

export function repairIntrinsics(
?
Or is it provided as a parameter to lockdown, such that it runs the shims when lockdown calls it back?

@kriskowal
Copy link
Member

I’m imagining usage like:

// start-lockdown-ses.js
repair();
// finish-ses-lockdown.js
lockdown();
// vetted-shim-lockdown.js
lockdown.pleaseKeepThisFunctionAroundTBD(Array.prototype, 'includes');
// ses-lockdown.js
import 'ses';
import './start-ses-lockdown.js';
import './vetted-shim.js';
import './vetted-shim-lockdown.js';
import './finish-ses-lockdown.js';

@erights erights added the permits Issues pertaining to SES’s permits for properties of shared intrinsics label Jun 25, 2021
@dckc dckc removed their assignment Jul 22, 2021
@Jack-Works
Copy link
Contributor

I believe this becomes urgent. SES lockdown now fails on core-js 3.23 and it cannot be fixed on the core-js side.

See discussion in: zloirock/core-js#1092

@kriskowal
Copy link
Member

We have answered this question with the separation of repairIntrinsics() and hardenIntrinsics(). Vetted shims can run between these phases and any additions to the shared intrinsics will be preserved.

This should get us out of most situations. We might need a mechanism in the future to add new shared intrinsics or behavior for creating new per-compartment intrinsics, which we can track separately when we find a motivating case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
permits Issues pertaining to SES’s permits for properties of shared intrinsics
Projects
None yet
Development

No branches or pull requests

4 participants