refactor(ses)!: Divide scope proxy into four layers
*BREAKING CHANGE*: By breaking the scope proxy into four separate
layers, SES will no longer leak scope proxies and consequently has
removed the knownScopeProxies experimental feature.
MODIFY
packages/ses/NEWS.md
packages/ses/NEWS.md
User-visible changes in SES:
+# Next release
+
+- Previous versions of SES would leak the proxy used to isolate evaluated
+ code to functions added to the global object by guest code.
+ The value of `this` in such functions should be `undefined`, but that is not
+ possible to emulate in this shim.
+ This version changes the value of `this` in such functions to be the same as
+ `globalThis` of the compartment, as would be correct in sloppy mode.
+- Removes experimental support for "known scope proxies".
+
# v0.16.0 (2022-10-19)
- When hardening a typed array, detects and locks down properties named as

MODIFY
packages/ses/src/compartment-evaluate.js
packages/ses/src/compartment-evaluate.js
@@ -28,11 +28,7 @@ export const provideCompartmentEvaluator = (compartmentFields, options) => {
// shared evaluator so we need to build a new one
let { globalTransforms } = compartmentFields;
- const {
- globalObject,
- globalLexicals,
- knownScopeProxies,
- } = compartmentFields;
+ const { globalObject, globalLexicals } = compartmentFields;
let localObject = globalLexicals;
if (__moduleShimLexicals__ !== undefined) {
@@ -58,7 +54,6 @@ export const provideCompartmentEvaluator = (compartmentFields, options) => {
globalLexicals: localObject,
globalTransforms,
sloppyGlobalsMode,
- knownScopeProxies,
}));
}

MODIFY
packages/ses/src/compartment-shim.js
packages/ses/src/compartment-shim.js
@@ -7,7 +7,6 @@ import {
ReferenceError,
TypeError,
WeakMap,
- WeakSet,
arrayFilter,
arrayJoin,
assign,
@@ -18,7 +17,6 @@ import {
promiseThen,
weakmapGet,
weakmapSet,
- weaksetHas,
} from './commons.js';
import {
setGlobalObjectConstantProperties,
@@ -119,12 +117,6 @@ export const CompartmentPrototype = {
return '[object Compartment]';
},
- /* eslint-disable-next-line no-underscore-dangle */
- __isKnownScopeProxy__(value) {
- const { knownScopeProxies } = weakmapGet(privateFields, this);
- return weaksetHas(knownScopeProxies, value);
- },
-
module(specifier) {
if (typeof specifier !== 'string') {
throw new TypeError('first argument of module() must be a string');
@@ -284,13 +276,11 @@ export const makeCompartmentConstructor = (
// evaluator is no longer eagerly created
setGlobalObjectConstantProperties(globalObject);
- const knownScopeProxies = new WeakSet();
const { safeEvaluate } = makeSafeEvaluator({
globalObject,
globalLexicals,
globalTransforms,
sloppyGlobalsMode: false,
- knownScopeProxies,
});
setGlobalObjectMutableProperties(globalObject, {
@@ -313,7 +303,6 @@ export const makeCompartmentConstructor = (
name: `${name}`,
globalTransforms,
globalObject,
- knownScopeProxies,
globalLexicals,
safeEvaluate,
resolveHook,

ADD
packages/ses/src/eval-scope.js
+import { FERAL_EVAL, create, freeze } from './commons.js';
+
+export const createEvalScope = () => {
+ const evalScope = create(null);
+ const oneTimeEvalProperties = freeze({
+ eval: {
+ get() {
+ delete evalScope.eval;
+ return FERAL_EVAL;
+ },
+ enumerable: false,
+ configurable: true,
+ },
+ });
+
+ return {
+ evalScope,
+ oneTimeEvalProperties,
+ };
+};

MODIFY
packages/ses/src/make-evaluate-factory.js
packages/ses/src/make-evaluate-factory.js
@@ -12,7 +12,7 @@ function buildOptimizer(constants) {
if (constants.length === 0) return '';
// Use 'this' to avoid going through the scope proxy, which is unecessary
// since the optimizer only needs references to the safe global.
- return `const {${arrayJoin(constants, ',')}} = this;`;
+ return `const {${arrayJoin(constants, ',')}} = this.optimizerObject;`;
}
/**
@@ -60,12 +60,18 @@ export const makeEvaluateFactory = (constants = []) => {
// We could probably both move the optimizer into the inner function
// and we could also simplify makeEvaluateFactory to simply evaluate.
return FERAL_FUNCTION(`
- with (this) {
+ with (this.scopeTerminator) {
+ with (this.globalObject) {
+ with (this.globalLexicals) {
+ with (this.evalScope) {
${optimizer}
return function() {
'use strict';
return eval(arguments[0]);
};
}
+ }
+ }
+ }
`);
};

MODIFY
packages/ses/src/make-safe-evaluator.js
packages/ses/src/make-safe-evaluator.js
// https://github.com/v8/v8/blob/master/src/builtins/builtins-function.cc
import {
- WeakSet,
apply,
- immutableObject,
- proxyRevocable,
- weaksetAdd,
+ create,
+ assign,
+ freeze,
+ defineProperties,
+ getOwnPropertyDescriptors,
} from './commons.js';
import { getScopeConstants } from './scope-constants.js';
-import { createScopeHandler } from './scope-handler.js';
+import { strictScopeTerminator } from './strict-scope-terminator.js';
+import { createSloppyGlobalsScopeTerminator } from './sloppy-globals-scope-terminator.js';
+import { createEvalScope } from './eval-scope.js';
import { applyTransforms, mandatoryTransforms } from './transforms.js';
import { makeEvaluateFactory } from './make-evaluate-factory.js';
import { assert } from './error/assert.js';
@@ -26,27 +29,17 @@ const { details: d } = assert;
* @param {Object} [options.globalLexicals]
* @param {Array<Transform>} [options.globalTransforms]
* @param {bool} [options.sloppyGlobalsMode]
- * @param {WeakSet} [options.knownScopeProxies]
*/
export const makeSafeEvaluator = ({
globalObject,
globalLexicals = {},
globalTransforms = [],
sloppyGlobalsMode = false,
- knownScopeProxies = new WeakSet(),
} = {}) => {
- const { scopeHandler, scopeController } = createScopeHandler(
- globalObject,
- globalLexicals,
- {
- sloppyGlobalsMode,
- },
- );
- const { proxy: scopeProxy, revoke: revokeScopeProxy } = proxyRevocable(
- immutableObject,
- scopeHandler,
- );
- weaksetAdd(knownScopeProxies, scopeProxy);
+ const scopeTerminator = sloppyGlobalsMode
+ ? createSloppyGlobalsScopeTerminator(globalObject)
+ : strictScopeTerminator;
+ const { evalScope, oneTimeEvalProperties } = createEvalScope();
// Defer creating the actual evaluator to first use.
// Creating a compartment should be possible in no-eval environments
@@ -56,7 +49,24 @@ export const makeSafeEvaluator = ({
if (!evaluate) {
const constants = getScopeConstants(globalObject, globalLexicals);
const evaluateFactory = makeEvaluateFactory(constants);
- evaluate = apply(evaluateFactory, scopeProxy, []);
+ const optimizerObject = defineProperties(
+ create(null),
+ assign(
+ getOwnPropertyDescriptors(globalObject),
+ getOwnPropertyDescriptors(globalLexicals),
+ ),
+ );
+ evaluate = apply(
+ evaluateFactory,
+ freeze({
+ optimizerObject,
+ evalScope,
+ globalLexicals,
+ globalObject,
+ scopeTerminator,
+ }),
+ [],
+ );
}
};
@@ -76,7 +86,11 @@ export const makeSafeEvaluator = ({
mandatoryTransforms,
]);
- scopeController.allowNextEvalToBeUnsafe = true;
+ // Allow next reference to eval produce the unsafe FERAL_EVAL.
+ // We avoid defineProperty because it consumes an extra stack frame taming
+ // its return value.
+ defineProperties(evalScope, oneTimeEvalProperties);
+
let err;
try {
// Ensure that "this" resolves to the safe global.
@@ -86,15 +100,14 @@ export const makeSafeEvaluator = ({
err = e;
throw e;
} finally {
- const unsafeEvalWasStillExposed = scopeController.allowNextEvalToBeUnsafe;
- scopeController.allowNextEvalToBeUnsafe = false;
+ const unsafeEvalWasStillExposed = 'eval' in evalScope;
+ delete evalScope.eval;
if (unsafeEvalWasStillExposed) {
- // Barring a defect in the SES shim, the scope proxy should allow the
+ // Barring a defect in the SES shim, the evalScope 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 we set
- // `scopeController.allowNextEvalToBeUnsafe` and before `evaluate`
- // calls `eval` internally.
+ // `evalScope.eval` 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.
@@ -104,7 +117,6 @@ export const makeSafeEvaluator = ({
// variable resolution via the scopeHandler, and throw an error with
// diagnostic info including the thrown error if any from evaluating the
// source code.
- revokeScopeProxy();
// TODO A GOOD PLACE TO PANIC(), i.e., kill the vat incarnation.
// See https://github.com/Agoric/SES-shim/issues/490
// eslint-disable-next-line @endo/no-polymorphic-call

MODIFY
packages/ses/src/scope-constants.js
packages/ses/src/scope-constants.js
@@ -160,7 +160,7 @@ export const getScopeConstants = (globalObject, localObject = {}) => {
);
// Collect all valid & immutable identifiers from the global that
- // are also not present in the endwoments (immutable or not).
+ // are also not present in the endowments (immutable or not).
const globalConstants = arrayFilter(
globalNames,
name =>

DELETE
packages/ses/src/scope-handler.js
-import {
- FERAL_EVAL,
- Proxy,
- String,
- TypeError,
- create,
- freeze,
- getOwnPropertyDescriptor,
- getOwnPropertyDescriptors,
- globalThis,
- immutableObject,
- objectHasOwnProperty,
- reflectGet,
- reflectSet,
- seal,
-} from './commons.js';
-import { assert } from './error/assert.js';
-
-const { details: d, quote: q } = assert;
-
-/**
- * alwaysThrowHandler
- * This is an object that throws if any property is called. It's used as
- * a proxy handler which throws on any trap called.
- * It's made from a proxy with a get trap that throws. It's safe to
- * create one and share it between all scopeHandlers.
- */
-const alwaysThrowHandler = new Proxy(
- immutableObject,
- freeze({
- get(_shadow, prop) {
- // eslint-disable-next-line @endo/no-polymorphic-call
- assert.fail(
- d`Please report unexpected scope handler trap: ${q(String(prop))}`,
- );
- },
- }),
-);
-
-/*
- * createScopeHandler()
- * ScopeHandler manages a Proxy which serves as the global scope for the
- * safeEvaluate operation (the Proxy is the argument of a 'with' binding).
- * As described in createSafeEvaluator(), it has several functions:
- * - allow the very first (and only the very first) use of 'eval' to map to
- * the real (unsafe) eval function, so it acts as a 'direct eval' and can
- * access its lexical scope (which maps to the 'with' binding, which the
- * ScopeHandler also controls).
- * - ensure that all subsequent uses of 'eval' map to the safeEvaluator,
- * which lives as the 'eval' property of the safeGlobal.
- * - route all other property lookups at the safeGlobal.
- * - hide the unsafeGlobal which lives on the scope chain above the 'with'.
- * - ensure the Proxy invariants despite some global properties being frozen.
- */
-export const createScopeHandler = (
- globalObject,
- globalLexicals = {},
- { sloppyGlobalsMode = false } = {},
-) => {
- // 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.
- // Using a flag on an object with a single mutable property allows a safe
- // evaluator to signal to the scope proxy without consuming a stack frame.
- // Consuming a stack frame could possibly allow an attacker to control the
- // stack depth before calling `evaluate` to cause a RangeError before this
- // flag can be reset, leaving the unsafe evaluator available.
- const scopeController = {
- allowNextEvalToBeUnsafe: false,
- };
- seal(scopeController);
-
- const scopeProxyHandlerProperties = {
- get(_shadow, prop) {
- if (typeof prop === 'symbol') {
- return undefined;
- }
-
- // Special treatment for eval. The very first lookup of 'eval' gets the
- // unsafe (real direct) eval, so it will get the lexical scope that uses
- // the 'with' context.
- if (prop === 'eval') {
- // test that it is true rather than merely truthy
- if (scopeController.allowNextEvalToBeUnsafe === true) {
- // revoke before use
- scopeController.allowNextEvalToBeUnsafe = false;
- return FERAL_EVAL;
- }
- // fall through
- }
-
- // Properties of the globalLexicals.
- if (prop in globalLexicals) {
- // Use reflect to defeat accessors that could be present on the
- // globalLexicals object itself as `this`.
- // This is done out of an overabundance of caution, as the SES shim
- // only use the globalLexicals carry globalLexicals and live binding
- // traps.
- // The globalLexicals are captured as a snapshot of what's passed to
- // the Compartment constructor, wherein all accessors and setters are
- // eliminated and the result frozen.
- // The live binding traps do use accessors, and none of those accessors
- // make use of their receiver.
- // Live binding traps provide no avenue for user code to observe the
- // receiver.
- return reflectGet(globalLexicals, prop, globalObject);
- }
-
- // Properties of the global.
- return reflectGet(globalObject, prop);
- },
-
- set(_shadow, prop, value) {
- // Properties of the globalLexicals.
- if (prop in globalLexicals) {
- const desc = getOwnPropertyDescriptor(globalLexicals, prop);
- if (objectHasOwnProperty(desc, 'value')) {
- // Work around a peculiar behavior in the specs, where
- // value properties are defined on the receiver.
- return reflectSet(globalLexicals, prop, value);
- }
- // Ensure that the 'this' value on setters resolves
- // to the safeGlobal, not to the globalLexicals object.
- return reflectSet(globalLexicals, prop, value, globalObject);
- }
-
- // Properties of the global.
- return reflectSet(globalObject, prop, value);
- },
-
- // we need has() to return false for some names to prevent the lookup from
- // climbing the scope chain and eventually reaching the unsafeGlobal
- // object (globalThis), which is bad.
-
- // todo: we'd like to just have has() return true for everything, and then
- // use get() to raise a ReferenceError for anything not on the safe global.
- // But we want to be compatible with ReferenceError in the normal case and
- // the lack of ReferenceError in the 'typeof' case. Must either reliably
- // distinguish these two cases (the trap behavior might be different), or
- // we rely on a mandatory source-to-source transform to change 'typeof abc'
- // to XXX. We already need a mandatory parse to prevent the 'import',
- // since it's a special form instead of merely being a global variable/
-
- // note: if we make has() return true always, then we must implement a
- // set() trap to avoid subverting the protection of strict mode (it would
- // accept assignments to undefined globals, when it ought to throw
- // ReferenceError for such assignments)
-
- has(_shadow, prop) {
- // unsafeGlobal: hide all properties of the current global
- // at the expense of 'typeof' being wrong for those properties. For
- // example, in the browser, evaluating 'document = 3', will add
- // a property to globalObject instead of throwing a ReferenceError.
-
- // !!!!! WARNING: DANGER ZONE !!!!!!
- // The order of the conditions in the `||` expression below is of the
- // utmost importance. Under no circumstances should `eval` be checked
- // after `globalObject`. The prototype of the global object is under
- // full control of user code and may be replaced by a proxy with a
- // `has` trap. If we allow that trap to trigger while the
- // `allowNextEvalToBeUnsafe` flag is down, it could allow user code
- // to get a hold of `FERAL_EVAL`, resulting in a complete escape of
- // the compartment.
- // !!!!! WARNING: DANGER ZONE !!!!!!
- return (
- sloppyGlobalsMode ||
- (scopeController.allowNextEvalToBeUnsafe && prop === 'eval') ||
- prop in globalLexicals ||
- prop in globalObject ||
- prop in globalThis
- );
- },
-
- // note: this is likely a bug of safari
- // https://bugs.webkit.org/show_bug.cgi?id=195534
-
- getPrototypeOf() {
- return null;
- },
-
- // Chip has seen this happen single stepping under the Chrome/v8 debugger.
- // TODO record how to reliably reproduce, and to test if this fix helps.
- // TODO report as bug to v8 or Chrome, and record issue link here.
-
- getOwnPropertyDescriptor(_target, prop) {
- // Coerce with `String` in case prop is a symbol.
- const quotedProp = q(String(prop));
- // eslint-disable-next-line @endo/no-polymorphic-call
- console.warn(
- `getOwnPropertyDescriptor trap on scopeHandler for ${quotedProp}`,
- new TypeError().stack,
- );
- return undefined;
- },
- };
-
- // The scope handler's prototype is a proxy that throws if any trap other
- // than get/set/has are run (like getOwnPropertyDescriptors, apply,
- // getPrototypeOf).
- const scopeHandler = freeze(
- create(
- alwaysThrowHandler,
- getOwnPropertyDescriptors(scopeProxyHandlerProperties),
- ),
- );
-
- return {
- scopeController,
- scopeHandler,
- };
-};

ADD
packages/ses/src/sloppy-globals-scope-terminator.js
+import {
+ Proxy,
+ create,
+ freeze,
+ getOwnPropertyDescriptors,
+ immutableObject,
+ reflectSet,
+} from './commons.js';
+import {
+ strictScopeTerminatorHandler,
+ alwaysThrowHandler,
+} from './strict-scope-terminator.js';
+
+/*
+ * createSloppyGlobalsScopeTerminator()
+ * strictScopeTerminatorHandler manages a scopeTerminator Proxy which serves as
+ * the final scope boundary that will always return "undefined" in order
+ * to prevent access to "start compartment globals". When "sloppyGlobalsMode"
+ * is true, the Proxy will perform sets on the "globalObject".
+ */
+export const createSloppyGlobalsScopeTerminator = globalObject => {
+ const scopeProxyHandlerProperties = {
+ // inherit scopeTerminator behavior
+ ...strictScopeTerminatorHandler,
+
+ // Redirect set properties to the globalObject.
+ set(_shadow, prop, value) {
+ return reflectSet(globalObject, prop, value);
+ },
+
+ // Always claim to have a potential property in order to be the recipient of a set
+ has(_shadow, _prop) {
+ return true;
+ },
+ };
+
+ // The scope handler's prototype is a proxy that throws if any trap other
+ // than get/set/has are run (like getOwnPropertyDescriptors, apply,
+ // getPrototypeOf).
+ const sloppyGlobalsScopeTerminatorHandler = freeze(
+ create(
+ alwaysThrowHandler,
+ getOwnPropertyDescriptors(scopeProxyHandlerProperties),
+ ),
+ );
+
+ const sloppyGlobalsScopeTerminator = new Proxy(
+ immutableObject,
+ sloppyGlobalsScopeTerminatorHandler,
+ );
+
+ return sloppyGlobalsScopeTerminator;
+};

ADD
packages/ses/src/strict-scope-terminator.js
+import {
+ Proxy,
+ String,
+ TypeError,
+ ReferenceError,
+ create,
+ freeze,
+ getOwnPropertyDescriptors,
+ globalThis,
+ immutableObject,
+} from './commons.js';
+import { assert } from './error/assert.js';
+
+const { details: d, quote: q } = assert;
+
+/**
+ * alwaysThrowHandler
+ * This is an object that throws if any property is called. It's used as
+ * a proxy handler which throws on any trap called.
+ * It's made from a proxy with a get trap that throws. It's safe to
+ * create one and share it between all Proxy handlers.
+ */
+export const alwaysThrowHandler = new Proxy(
+ immutableObject,
+ freeze({
+ get(_shadow, prop) {
+ // eslint-disable-next-line @endo/no-polymorphic-call
+ assert.fail(
+ d`Please report unexpected scope handler trap: ${q(String(prop))}`,
+ );
+ },
+ }),
+);
+
+/*
+ * scopeProxyHandlerProperties
+ * scopeTerminatorHandler manages a strictScopeTerminator Proxy which serves as
+ * the final scope boundary that will always return "undefined" in order
+ * to prevent access to "start compartment globals".
+ */
+const scopeProxyHandlerProperties = {
+ get(_shadow, _prop) {
+ return undefined;
+ },
+
+ set(_shadow, prop, _value) {
+ // We should only hit this if the has() hook returned true matches the v8
+ // ReferenceError message "Uncaught ReferenceError: xyz is not defined"
+ throw new ReferenceError(`${String(prop)} is not defined`);
+ },
+
+ has(_shadow, prop) {
+ // we must at least return true for all properties on the realm globalThis
+ return prop in globalThis;
+ },
+
+ // note: this is likely a bug of safari
+ // https://bugs.webkit.org/show_bug.cgi?id=195534
+ getPrototypeOf() {
+ return null;
+ },
+
+ // Chip has seen this happen single stepping under the Chrome/v8 debugger.
+ // TODO record how to reliably reproduce, and to test if this fix helps.
+ // TODO report as bug to v8 or Chrome, and record issue link here.
+ getOwnPropertyDescriptor(_target, prop) {
+ // Coerce with `String` in case prop is a symbol.
+ const quotedProp = q(String(prop));
+ // eslint-disable-next-line @endo/no-polymorphic-call
+ console.warn(
+ `getOwnPropertyDescriptor trap on scopeTerminatorHandler for ${quotedProp}`,
+ new TypeError().stack,
+ );
+ return undefined;
+ },
+};
+
+// The scope handler's prototype is a proxy that throws if any trap other
+// than get/set/has are run (like getOwnPropertyDescriptors, apply,
+// getPrototypeOf).
+export const strictScopeTerminatorHandler = freeze(
+ create(
+ alwaysThrowHandler,
+ getOwnPropertyDescriptors(scopeProxyHandlerProperties),
+ ),
+);
+
+export const strictScopeTerminator = new Proxy(
+ immutableObject,
+ strictScopeTerminatorHandler,
+);

MODIFY
packages/ses/src/whitelist.js
packages/ses/src/whitelist.js
@@ -1361,7 +1361,6 @@ export const whitelist = {
name: getter,
// Should this be proposed?
toString: fn,
- __isKnownScopeProxy__: fn,
import: asyncFn,
load: asyncFn,
importNow: fn,

refactor(ses)!: Divide scope proxy into four layers

*BREAKING CHANGE*: By breaking the scope proxy into four separate
layers, SES will no longer leak scope proxies and consequently has
removed the knownScopeProxies experimental feature.