test(ses): cover the allowNextEvalToBeUnsafe failure after stack overflow

packages/ses/src/commons.js

@@ -129,7 +129,7 @@ export const {
   set: reflectSet,
 } = Reflect;
 
-export const { isArray, prototype: arrayPrototype } = Array;
+export const { isArray, prototype: arrayPrototype, from: ArrayFrom } = Array;
 export const { prototype: arrayBufferPrototype } = ArrayBuffer;
 export const { prototype: mapPrototype } = Map;
 export const { revocable: proxyRevocable } = Proxy;

packages/ses/src/eval-scope.js

@@ -1,4 +1,10 @@
-import { FERAL_EVAL, create, defineProperties, freeze } from './commons.js';
+import {
+  FERAL_EVAL,
+  apply,
+  create,
+  defineProperties,
+  freeze,
+} from './commons.js';
 
 import { assert } from './error/assert.js';
 
@@ -69,17 +75,35 @@ export const makeEvalScopeKit = () => {
     },
   });
 
+  const allowEvalProperties = freeze({
+    eval: {
+      value: props => {
+        const { eval: evalProp } = props;
+        delete evalScope.eval;
+        defineProperties(evalScope, { eval: evalProp });
+      },
+      enumerable: false,
+      configurable: true,
+    },
+  });
+
   const evalScopeKit = {
     evalScope,
-    allowNextEvalToBeUnsafe() {
+    allowNextEvalToBeUnsafe(evaluate, globalObject) {
       const { revoked } = evalScopeKit;
       if (revoked !== null) {
         Fail`a handler did not reset allowNextEvalToBeUnsafe ${revoked.err}`;
       }
-      // Allow next reference to eval produce the unsafe FERAL_EVAL.
+      // Use the evaluator and evalScope to reveal the unsafe FERAL_EVAL
       // We avoid defineProperty because it consumes an extra stack frame taming
       // its return value.
-      defineProperties(evalScope, oneTimeEvalProperties);
+      defineProperties(evalScope, allowEvalProperties);
+      try {
+        apply(evaluate, globalObject, [oneTimeEvalProperties]);
+      } catch (err) {
+        // We didn't manage to reveal eval, so clean up the powerless revealer
+        delete evalScope.eval;
+      }
     },
     /** @type {null | { err: any }} */
     revoked: null,

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

@@ -32,7 +32,7 @@ export const makeSafeEvaluator = ({
     ? createSloppyGlobalsScopeTerminator(globalObject)
     : strictScopeTerminator;
   const evalScopeKit = makeEvalScopeKit();
-  const { evalScope } = evalScopeKit;
+  const { evalScope, allowNextEvalToBeUnsafe } = evalScopeKit;
 
   const evaluateContext = freeze({
     evalScope,
@@ -73,8 +73,7 @@ export const makeSafeEvaluator = ({
     let err;
     try {
       // Allow next reference to eval produce the unsafe FERAL_EVAL.
-      // eslint-disable-next-line @endo/no-polymorphic-call
-      evalScopeKit.allowNextEvalToBeUnsafe();
+      allowNextEvalToBeUnsafe(evaluate, globalObject);
 
       // Ensure that "this" resolves to the safe global.
       return apply(evaluate, globalObject, [source]);

packages/ses/test/eval-scope.test.js

@@ -12,10 +12,11 @@ test('evalScope - is not created with eval exposed', t => {
 
   const evalScopeKit = makeEvalScopeKit();
   const { evalScope } = evalScopeKit;
+  const evaluateForAllower = props => evalScope.eval(props);
 
   t.is(Reflect.has(evalScope, 'eval'), false);
 
-  evalScopeKit.allowNextEvalToBeUnsafe();
+  evalScopeKit.allowNextEvalToBeUnsafe(evaluateForAllower);
 
   t.is(Reflect.has(evalScope, 'eval'), true);
 });
@@ -25,10 +26,11 @@ test('evalScope - getting eval removes it from evalScope', t => {
 
   const evalScopeKit = makeEvalScopeKit();
   const { evalScope } = evalScopeKit;
+  const evaluateForAllower = props => evalScope.eval(props);
 
   t.is(Reflect.has(evalScope, 'eval'), false);
 
-  evalScopeKit.allowNextEvalToBeUnsafe();
+  evalScopeKit.allowNextEvalToBeUnsafe(evaluateForAllower);
 
   t.is(Reflect.has(evalScope, 'eval'), true);
 

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

@@ -47,7 +47,7 @@ test('makeEvaluate - optimizer', t => {
   globalObjectOps.length = 0;
   moduleLexicalsOps.length = 0;
 
-  evalScopeKit.allowNextEvalToBeUnsafe();
+  evalScopeKit.allowNextEvalToBeUnsafe(evaluate, globalObject);
 
   const result = apply(evaluate, globalObject, [`!foo && bar && baz`]);
 
@@ -70,9 +70,9 @@ test('makeEvaluate - strict-mode', t => {
     freeze({ scopeTerminator, globalObject, moduleLexicals, evalScope }),
   );
 
-  evalScopeKit.allowNextEvalToBeUnsafe();
+  evalScopeKit.allowNextEvalToBeUnsafe(evaluate, globalObject);
   t.throws(() => apply(evaluate, globalObject, [`foo = 42`]));
 
-  evalScopeKit.allowNextEvalToBeUnsafe();
+  evalScopeKit.allowNextEvalToBeUnsafe(evaluate, globalObject);
   t.throws(() => apply(evaluate, globalObject, [`with({}) {}`]));
 });

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

@@ -115,3 +115,30 @@ test('safeEvaluate - transforms - rewrite source', t => {
     'localTransforms rewrite source first',
   );
 });
+
+test.failing(`safeEvaluate - stack overflow doesn't leak feral eval`, t => {
+  t.plan(3);
+  const globalObject = Object.create(null);
+  const moduleLexicals = Object.create(null);
+
+  const { safeEvaluate: evaluate } = makeSafeEvaluator({
+    globalObject,
+    moduleLexicals,
+  });
+
+  let depth = 0;
+  function stackBuildUp() {
+    depth += 1;
+    evaluate(`1+1`);
+    // didn't fail yet? keep digging.
+    return stackBuildUp();
+  }
+
+  t.throws(stackBuildUp, {
+    instanceOf: RangeError,
+    message: /Maximum call stack size exceeded/,
+  });
+  t.log('stack depth', depth);
+  t.is(depth > 1, true, 'stack overflow occurred after many recursions');
+  t.is(evaluate('1+1'), 2, `stack overflow didn't compromise evaluate scope`);
+});