fix(ses): Patch leak of globalLexicals thru moduleLexicals

packages/ses/NEWS.md

@@ -1,5 +1,10 @@
 User-visible changes in SES:
 
+# Next release
+
+- *SECURITY FIX*: Closes a leak for `globalLexicals`, such that a module
+  lexical scope cannot be used to reveal names in global lexical scope.
+
 # v0.17.0 (2022-10-24)
 
 - Previous versions of SES would leak the proxy used to isolate evaluated

packages/ses/src/compartment-evaluate.js

@@ -3,7 +3,6 @@ import {
   TypeError,
   arrayPush,
   create,
-  defineProperties,
   getOwnPropertyDescriptors,
 } from './commons.js';
 import {
@@ -30,8 +29,11 @@ export const provideCompartmentEvaluator = (compartmentFields, options) => {
     let { globalTransforms } = compartmentFields;
     const { globalObject, globalLexicals } = compartmentFields;
 
-    let localObject = globalLexicals;
-    if (__moduleShimLexicals__ !== undefined) {
+    let moduleLexicals;
+    if (
+      __moduleShimLexicals__ !== undefined &&
+      __moduleShimLexicals__ !== null
+    ) {
       // When using `evaluate` for ESM modules, as should only occur from the
       // module-shim's module-instance.js, we do not reveal the SES-shim's
       // module-to-program translation, as this is not standardizable behavior.
@@ -42,16 +44,16 @@ export const provideCompartmentEvaluator = (compartmentFields, options) => {
       // and `import`, at the expense of being tightly coupled to SES-shim.
       globalTransforms = undefined;
 
-      localObject = create(null, getOwnPropertyDescriptors(globalLexicals));
-      defineProperties(
-        localObject,
+      moduleLexicals = create(
+        null,
         getOwnPropertyDescriptors(__moduleShimLexicals__),
       );
     }
 
     ({ safeEvaluate } = makeSafeEvaluator({
       globalObject,
-      globalLexicals: localObject,
+      globalLexicals,
+      moduleLexicals,
       globalTransforms,
       sloppyGlobalsMode,
     }));

packages/ses/src/make-evaluate.js

@@ -26,14 +26,20 @@ function buildOptimizer(constants, name) {
  *
  * @param {object} context
  * @param {object} context.evalScope
- * @param {object} context.globalLexicals
  * @param {object} context.globalObject
+ * @param {object} context.globalLexicals
+ * @param {object} context.moduleLexicals
  * @param {object} context.scopeTerminator
  */
 export const makeEvaluate = context => {
-  const { globalObjectConstants, globalLexicalConstants } = getScopeConstants(
+  const {
+    globalObjectConstants,
+    globalLexicalConstants,
+    moduleLexicalConstants,
+  } = getScopeConstants(
     context.globalObject,
     context.globalLexicals,
+    context.moduleLexicals,
   );
   const globalObjectOptimizer = buildOptimizer(
     globalObjectConstants,
@@ -43,6 +49,10 @@ export const makeEvaluate = context => {
     globalLexicalConstants,
     'globalLexicals',
   );
+  const moduleLexicalOptimizer = buildOptimizer(
+    moduleLexicalConstants,
+    'moduleLexicals',
+  );
 
   // Create a function in sloppy mode, so that we can use 'with'. It returns
   // a function in strict mode that evaluates the provided code using direct
@@ -93,13 +103,16 @@ export const makeEvaluate = context => {
     with (this.scopeTerminator) {
       with (this.globalObject) {
         with (this.globalLexicals) {
-          with (this.evalScope) {
-            ${globalObjectOptimizer}
-            ${globalLexicalOptimizer}
-            return function() {
-              'use strict';
-              return eval(arguments[0]);
-            };
+          with (this.moduleLexicals) {
+            with (this.evalScope) {
+              ${globalObjectOptimizer}
+              ${globalLexicalOptimizer}
+              ${moduleLexicalOptimizer}
+              return function() {
+                'use strict';
+                return eval(arguments[0]);
+              };
+            }
           }
         }
       }

packages/ses/src/make-safe-evaluator.js

@@ -21,10 +21,12 @@ const { details: d } = assert;
  * @param {Object} [options.globalLexicals]
  * @param {Array<Transform>} [options.globalTransforms]
  * @param {bool} [options.sloppyGlobalsMode]
+ * @param {Object} [options.moduleLexicals]
  */
 export const makeSafeEvaluator = ({
   globalObject,
   globalLexicals = {},
+  moduleLexicals = {},
   globalTransforms = [],
   sloppyGlobalsMode = false,
 } = {}) => {
@@ -36,6 +38,7 @@ export const makeSafeEvaluator = ({
 
   const evaluateContext = freeze({
     evalScope,
+    moduleLexicals,
     globalLexicals,
     globalObject,
     scopeTerminator,

packages/ses/src/scope-constants.js

@@ -145,17 +145,32 @@ function isImmutableDataProperty(obj, name) {
  *
  * @param {Object} globalObject
  * @param {Object} globalLexicals
+ * @param {Object} moduleLexicals
  */
-export const getScopeConstants = (globalObject, globalLexicals = {}) => {
+export const getScopeConstants = (
+  globalObject,
+  globalLexicals = {},
+  moduleLexicals = {},
+) => {
   // getOwnPropertyNames() does ignore Symbols so we don't need to
   // filter them out.
   const globalObjectNames = getOwnPropertyNames(globalObject);
   const globalLexicalNames = getOwnPropertyNames(globalLexicals);
+  const moduleLexicalNames = getOwnPropertyNames(moduleLexicals);
+
+  // Collect all valid & immutable identifiers from the endowments.
+  const moduleLexicalConstants = arrayFilter(
+    moduleLexicalNames,
+    name =>
+      isValidIdentifierName(name) &&
+      isImmutableDataProperty(moduleLexicals, name),
+  );
 
   // Collect all valid & immutable identifiers from the endowments.
   const globalLexicalConstants = arrayFilter(
     globalLexicalNames,
     name =>
+      !arrayIncludes(moduleLexicalNames, name) &&
       isValidIdentifierName(name) &&
       isImmutableDataProperty(globalLexicals, name),
   );
@@ -167,6 +182,7 @@ export const getScopeConstants = (globalObject, globalLexicals = {}) => {
     name =>
       // Can't define a constant: it would prevent a
       // lookup on the endowments.
+      !arrayIncludes(moduleLexicalNames, name) &&
       !arrayIncludes(globalLexicalNames, name) &&
       isValidIdentifierName(name) &&
       isImmutableDataProperty(globalObject, name),
@@ -175,5 +191,6 @@ export const getScopeConstants = (globalObject, globalLexicals = {}) => {
   return {
     globalObjectConstants,
     globalLexicalConstants,
+    moduleLexicalConstants,
   };
 };

packages/ses/test/test-compartment.js

@@ -99,7 +99,7 @@ test('main use case', t => {
   t.throws(() => user(-1), { instanceOf: TypeError });
 });
 
-test.failing('compartment does not leak globalLexicals through moduleLexicals', t => {
+test('compartment does not leak globalLexicals through moduleLexicals', t => {
   const compartment = new Compartment(null, null, {
     globalLexicals: {
       secret: 'secret',

packages/ses/test/test-make-evaluate.js

@@ -19,32 +19,43 @@ const makeObservingProxy = target => {
 };
 
 test('makeEvaluate - optimizer', t => {
-  t.plan(5);
+  t.plan(6);
 
   const globalObjectTarget = Object.create(null, {
     foo: { value: true },
     bar: { value: true },
     baz: { value: true, writable: true },
   });
   const globalLexicalsTarget = Object.create(null, { foo: { value: false } });
+  const moduleLexicalsTarget = Object.create(null, { qux: { value: false } });
 
   const [globalObject, globalObjectOps] = makeObservingProxy(
     globalObjectTarget,
   );
   const [globalLexicals, globalLexicalsOps] = makeObservingProxy(
     globalLexicalsTarget,
   );
+  const [moduleLexicals, moduleLexicalsOps] = makeObservingProxy(
+    moduleLexicalsTarget,
+  );
 
   const scopeTerminator = strictScopeTerminator;
   const evalScopeKit = makeEvalScopeKit();
   const { evalScope } = evalScopeKit;
 
   const evaluate = makeEvaluate(
-    freeze({ scopeTerminator, globalObject, globalLexicals, evalScope }),
+    freeze({
+      scopeTerminator,
+      globalObject,
+      globalLexicals,
+      moduleLexicals,
+      evalScope,
+    }),
   );
 
   t.deepEqual(globalObjectOps, [['get', 'bar']]);
   t.deepEqual(globalLexicalsOps, [['get', 'foo']]);
+  t.deepEqual(moduleLexicalsOps, [['get', 'qux']]);
 
   globalObjectOps.length = 0;
   globalLexicalsOps.length = 0;
@@ -63,13 +74,20 @@ test('makeEvaluate - strict-mode', t => {
 
   const globalObject = Object.create(null);
   const globalLexicals = Object.create(null);
+  const moduleLexicals = Object.create(null);
 
   const scopeTerminator = strictScopeTerminator;
   const evalScopeKit = makeEvalScopeKit();
   const { evalScope } = evalScopeKit;
 
   const evaluate = makeEvaluate(
-    freeze({ scopeTerminator, globalObject, globalLexicals, evalScope }),
+    freeze({
+      scopeTerminator,
+      globalObject,
+      globalLexicals,
+      moduleLexicals,
+      evalScope,
+    }),
   );
 
   evalScopeKit.allowNextEvalToBeUnsafe();