refactor(ses): Avoid use of receiver object `this` in scope handler
MODIFY
packages/ses/src/evaluate.js
packages/ses/src/evaluate.js
@@ -49,17 +49,20 @@ export const performEval = (
mandatoryTransforms,
]);
- const scopeHandler = createScopeHandler(globalObject, localObject, {
+ const {
+ scopeHandler,
+ admitOneUnsafeEvalNext,
+ resetOneUnsafeEvalNext,
+ } = createScopeHandler(globalObject, localObject, {
sloppyGlobalsMode,
});
const scopeProxyRevocable = proxyRevocable(immutableObject, scopeHandler);
- // Ensure that "this" resolves to the scope proxy.
const constants = getScopeConstants(globalObject, localObject);
const evaluateFactory = makeEvaluateFactory(constants);
const evaluate = apply(evaluateFactory, scopeProxyRevocable.proxy, []);
- scopeHandler.useUnsafeEvaluator = true;
+ admitOneUnsafeEvalNext();
let err;
try {
// Ensure that "this" resolves to the safe global.
@@ -70,19 +73,26 @@ export const performEval = (
err = e;
throw e;
} finally {
- if (scopeHandler.useUnsafeEvaluator === true) {
- // The proxy switches off useUnsafeEvaluator immediately after
- // the first access, but if that's not the case we should abort.
- // This condition is one where this vat is now hopelessly confused,
- // and the vat as a whole should be aborted. All immediately reachable
- // state should be abandoned. However, that is not yet possible,
- // so we at least prevent further variable resolution via the
- // scopeHandler, and throw an error with diagnostic info including
- // the thrown error if any from evaluating the source code.
+ if (resetOneUnsafeEvalNext()) {
+ // Barring a defect in the SES shim, the scope proxy should allow the
+ // powerful, unsafe `eval` to be used by `evaluate` exactly once, as the
+ // very first name that it attempts to access from the lexical scope.
+ // A defect in the SES shim could throw an exception after our call to
+ // `admitOneUnsafeEvalNext()` and before `evaluate` calls `eval`
+ // internally.
+ // If we get here, SES is very broken.
+ // This condition is one where this vat is now hopelessly confused, and
+ // the vat as a whole should be aborted.
+ // No further code should run.
+ // All immediately reachable state should be abandoned.
+ // However, that is not yet possible, so we at least prevent further
+ // variable resolution via the scopeHandler, and throw an error with
+ // diagnostic info including the thrown error if any from evaluating the
+ // source code.
scopeProxyRevocable.revoke();
// TODO A GOOD PLACE TO PANIC(), i.e., kill the vat incarnation.
// See https://github.com/Agoric/SES-shim/issues/490
- assert.fail(d`handler did not revoke useUnsafeEvaluator ${err}`);
+ assert.fail(d`handler did not reset allowNextEvalToBeUnsafe ${err}`);
}
}
};

MODIFY
packages/ses/src/make-evaluate-factory.js
packages/ses/src/make-evaluate-factory.js
@@ -46,9 +46,10 @@ export const makeEvaluateFactory = (constants = []) => {
// - keywords like 'function' which are reserved keywords, and cannot be
// used as a variables, so they is not part to the optimizer.
// - when 'eval' is looked up in the proxy, and it's the first time it is
- // looked up after useUnsafeEvaluator is turned on, the proxy returns the
- // eval intrinsic, and flips useUnsafeEvaluator back to false. Any reference
- // to 'eval' in that string will get the tamed evaluator.
+ // looked up after allowNextEvalToBeUnsafe is turned on, the proxy returns
+ // the powerful, unsafe eval intrinsic, and flips allowNextEvalToBeUnsafe
+ // back to false. Any reference to 'eval' in that string will get the tamed
+ // evaluator.
// TODO https://github.com/endojs/endo/issues/816
// The optimizer currently runs under sloppy mode, and although we doubt that

MODIFY
packages/ses/src/scope-handler.js
packages/ses/src/scope-handler.js
@@ -3,6 +3,7 @@ import {
FERAL_EVAL,
Proxy,
String,
+ freeze,
getOwnPropertyDescriptor,
globalThis,
immutableObject,
@@ -48,15 +49,26 @@ export const createScopeHandler = (
localObject = {},
{ sloppyGlobalsMode = false } = {},
) => {
- return {
+ // This flag allow us to determine if the eval() call is an done by the
+ // compartment's code or if it is user-land invocation, so we can react
+ // differently.
+ let allowNextEvalToBeUnsafe = false;
+
+ const admitOneUnsafeEvalNext = () => {
+ allowNextEvalToBeUnsafe = true;
+ };
+
+ const resetOneUnsafeEvalNext = () => {
+ const wasSet = allowNextEvalToBeUnsafe;
+ allowNextEvalToBeUnsafe = false;
+ return wasSet;
+ };
+
+ const scopeHandler = freeze({
// The scope handler throws if any trap other than get/set/has are run
// (e.g. getOwnPropertyDescriptors, apply, getPrototypeOf).
__proto__: alwaysThrowHandler,
- // This flag allow us to determine if the eval() call is an done by the
- // realm's code or if it is user-land invocation, so we can react differently.
- useUnsafeEvaluator: false,
-
get(_shadow, prop) {
if (typeof prop === 'symbol') {
return undefined;
@@ -67,9 +79,9 @@ export const createScopeHandler = (
// the 'with' context.
if (prop === 'eval') {
// test that it is true rather than merely truthy
- if (this.useUnsafeEvaluator === true) {
+ if (allowNextEvalToBeUnsafe === true) {
// revoke before use
- this.useUnsafeEvaluator = false;
+ allowNextEvalToBeUnsafe = false;
return FERAL_EVAL;
}
// fall through
@@ -166,5 +178,11 @@ export const createScopeHandler = (
);
return undefined;
},
+ });
+
+ return {
+ admitOneUnsafeEvalNext,
+ resetOneUnsafeEvalNext,
+ scopeHandler,
};
};

MODIFY
packages/ses/test/test-scope-handler.js
packages/ses/test/test-scope-handler.js
@@ -17,7 +17,10 @@ test('scopeHandler - has trap', t => {
const globalObject = { foo: {} };
const endowments = { foobar: {} };
- const handler = createScopeHandler(globalObject, endowments);
+ const { scopeHandler: handler } = createScopeHandler(
+ globalObject,
+ endowments,
+ );
t.is(handler.has(null, Symbol.unscopables), false);
t.is(handler.has(null, 'arguments'), false);
@@ -37,7 +40,11 @@ test('scopeHandler - has trap in sloppyGlobalsMode', t => {
const globalObject = {};
const endowments = {};
const options = { sloppyGlobalsMode: true };
- const handler = createScopeHandler(globalObject, endowments, options);
+ const { scopeHandler: handler } = createScopeHandler(
+ globalObject,
+ endowments,
+ options,
+ );
globalThis.bar = {};
@@ -58,7 +65,10 @@ test('scopeHandler - get trap', t => {
const globalObject = { foo: {} };
const endowments = { foobar: {} };
- const handler = createScopeHandler(globalObject, endowments);
+ const { scopeHandler: handler } = createScopeHandler(
+ globalObject,
+ endowments,
+ );
globalThis.bar = {};
@@ -79,7 +89,10 @@ test('scopeHandler - get trap - accessors on endowments', t => {
const globalObject = { foo: {} };
const endowments = {};
- const handler = createScopeHandler(globalObject, endowments);
+ const { scopeHandler: handler } = createScopeHandler(
+ globalObject,
+ endowments,
+ );
Object.defineProperties(endowments, {
foo: {
@@ -98,7 +111,10 @@ test('scopeHandler - set trap', t => {
const globalObject = { foo: {} };
const endowments = { foobar: {} };
- const handler = createScopeHandler(globalObject, endowments);
+ const { scopeHandler: handler } = createScopeHandler(
+ globalObject,
+ endowments,
+ );
globalThis.bar = {};
@@ -137,19 +153,23 @@ test('scopeHandler - set trap', t => {
delete globalThis.bar;
});
-test('scopeHandler - get trap - useUnsafeEvaluator', t => {
+test('scopeHandler - get trap - reset allow next unsafe eval', t => {
t.plan(7);
const globalObject = { eval: {} };
- const handler = createScopeHandler(globalObject);
+ const {
+ scopeHandler: handler,
+ resetOneUnsafeEvalNext,
+ admitOneUnsafeEvalNext,
+ } = createScopeHandler(globalObject);
- t.is(handler.useUnsafeEvaluator, false);
+ t.is(resetOneUnsafeEvalNext(), false);
t.is(handler.get(null, 'eval'), globalObject.eval);
t.is(handler.get(null, 'eval'), globalObject.eval); // repeat
- handler.useUnsafeEvaluator = true;
+ admitOneUnsafeEvalNext();
t.is(handler.get(null, 'eval'), FERAL_EVAL);
- t.is(handler.useUnsafeEvaluator, false);
+ t.is(resetOneUnsafeEvalNext(), false);
t.is(handler.get(null, 'eval'), globalObject.eval);
t.is(handler.get(null, 'eval'), globalObject.eval); // repeat
});
@@ -160,7 +180,7 @@ test('scopeHandler - throw only for unsupported traps', t => {
sinon.stub(console, 'error').callsFake();
const globalObject = {};
- const handler = createScopeHandler(globalObject);
+ const { scopeHandler: handler } = createScopeHandler(globalObject);
['has', 'get', 'set', 'getPrototypeOf'].forEach(trap =>
t.notThrows(() => handler[trap]),

refactor(ses): Avoid use of receiver object `this` in scope handler