feat(ses): Hermes eval and compartment taming

leotm · leotm · commit 063ec3354564 · 2025-02-19T19:04:03.000Z
diff --git a/packages/ses/src/global-object.js b/packages/ses/src/global-object.js
@@ -19,7 +19,7 @@ import { constantProperties, universalPropertyNames } from './permits.js';
  * guest programs, we cannot emulate the proper behavior.
  * With this shim, assigning Symbol.unscopables causes the given lexical
  * names to fall through to the terminal scope proxy.
- * But, we can install this setter to prevent a program from proceding on
+ * But, we can install this setter to prevent a program from proceeding on
  * this false assumption.
  *
  * @param {object} globalObject
@@ -146,14 +146,16 @@ export const setGlobalObjectMutableProperties = (
  * @param {object} globalObject
  * @param {Function} evaluator
  * @param {(object) => void} markVirtualizedNativeFunction
+ * @param {string} [legacyHermesTaming]
  */
 export const setGlobalObjectEvaluators = (
   globalObject,
   evaluator,
   markVirtualizedNativeFunction,
+  legacyHermesTaming,
 ) => {
   {
-    const f = freeze(makeEvalFunction(evaluator));
+    const f = freeze(makeEvalFunction(evaluator, legacyHermesTaming));
     markVirtualizedNativeFunction(f);
     defineProperty(globalObject, 'eval', {
       value: f,
diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js
@@ -102,6 +102,7 @@ const safeHarden = makeHardener();
 
 const assertDirectEvalAvailable = () => {
   let allowed = false;
+  let evaluatorsBlocked = false;
   try {
     allowed = FERAL_FUNCTION(
       'eval',
@@ -122,12 +123,13 @@ const assertDirectEvalAvailable = () => {
     // We reach here if eval is outright forbidden by a Content Security Policy.
     // We allow this for SES usage that delegates the responsibility to isolate
     // guest code to production code generation.
-    allowed = true;
+    evaluatorsBlocked = true;
   }
-  if (!allowed) {
+  if (!allowed && !evaluatorsBlocked) {
     // See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_DIRECT_EVAL.md
     throw TypeError(
-      `SES cannot initialize unless 'eval' is the original intrinsic 'eval', suitable for direct-eval (dynamically scoped eval) (SES_DIRECT_EVAL)`,
+      `SES cannot initialize unless 'eval' is the original intrinsic 'eval', suitable for direct-eval (dynamically scoped eval) (SES_DIRECT_EVAL)
+Did you mean legacyHermesTaming: 'unsafe'?`,
     );
   }
 };
@@ -152,11 +154,11 @@ export const repairIntrinsics = (options = {}) => {
   // The `stackFiltering` is not a safety issue. Rather it is a tradeoff
   // between relevance and completeness of the stack frames shown on the
   // console. Setting`stackFiltering` to `'verbose'` applies no filters, providing
-  // the raw stack frames that can be quite versbose. Setting
+  // the raw stack frames that can be quite verbose. Setting
   // `stackFrameFiltering` to`'concise'` limits the display to the stack frame
   // information most likely to be relevant, eliminating distracting frames
   // such as those from the infrastructure. However, the bug you're trying to
-  // track down might be in the infrastrure, in which case the `'verbose'` setting
+  // track down might be in the infrastructure, in which case the `'verbose'` setting
   // is useful. See
   // [`stackFiltering` options](https://github.com/Agoric/SES-shim/blob/master/packages/ses/docs/lockdown.md#stackfiltering-options)
   // for an explanation.
@@ -189,6 +191,9 @@ export const repairIntrinsics = (options = {}) => {
       /** @param {string} debugName */
       debugName => debugName !== '',
     ),
+    legacyHermesTaming = /** @type { 'safe' | 'unsafe' } */ (
+      getenv('LOCKDOWN_LEGACY_HERMES_TAMING', 'safe')
+    ),
     legacyRegeneratorRuntimeTaming = getenv(
       'LOCKDOWN_LEGACY_REGENERATOR_RUNTIME_TAMING',
       'safe',
@@ -199,6 +204,10 @@ export const repairIntrinsics = (options = {}) => {
     ...extraOptions
   } = options;
 
+  legacyHermesTaming === 'safe' ||
+    legacyHermesTaming === 'unsafe' ||
+    Fail`lockdown(): non supported option legacyHermesTaming: ${q(legacyHermesTaming)}`;
+
   legacyRegeneratorRuntimeTaming === 'safe' ||
     legacyRegeneratorRuntimeTaming === 'unsafe-ignore' ||
     Fail`lockdown(): non supported option legacyRegeneratorRuntimeTaming: ${q(legacyRegeneratorRuntimeTaming)}`;
@@ -218,13 +227,11 @@ export const repairIntrinsics = (options = {}) => {
   const { warn } = reporter;
 
   if (dateTaming !== undefined) {
-    // eslint-disable-next-line no-console
     warn(
       `SES The 'dateTaming' option is deprecated and does nothing. In the future specifying it will be an error.`,
     );
   }
   if (mathTaming !== undefined) {
-    // eslint-disable-next-line no-console
     warn(
       `SES The 'mathTaming' option is deprecated and does nothing. In the future specifying it will be an error.`,
     );
@@ -242,7 +249,13 @@ export const repairIntrinsics = (options = {}) => {
   // trace retained:
   priorRepairIntrinsics.stack;
 
-  assertDirectEvalAvailable();
+  if (legacyHermesTaming === 'safe') {
+    assertDirectEvalAvailable();
+  } else if (legacyHermesTaming === 'unsafe') {
+    warn(
+      `SES initializing with an unoriginal intrinsic 'eval', not suitable for direct-eval (dynamically scoped eval) (SES_DIRECT_EVAL)`,
+    );
+  }
 
   /**
    * Because of packagers and bundlers, etc, multiple invocations of lockdown
@@ -408,6 +421,12 @@ export const repairIntrinsics = (options = {}) => {
     markVirtualizedNativeFunction,
   });
 
+  if (legacyHermesTaming === 'unsafe') {
+    globalThis.testCompartmentHooks = undefined;
+    // @ts-ignore Compartment does exist on globalThis
+    delete globalThis.Compartment;
+  }
+
   if (evalTaming === 'noEval') {
     setGlobalObjectEvaluators(
       globalThis,
@@ -420,6 +439,7 @@ export const repairIntrinsics = (options = {}) => {
       globalThis,
       safeEvaluate,
       markVirtualizedNativeFunction,
+      legacyHermesTaming,
     );
   } else if (evalTaming === 'unsafeEval') {
     // Leave eval function and Function constructor of the initial compartment in-tact.
diff --git a/packages/ses/src/make-eval-function.js b/packages/ses/src/make-eval-function.js
@@ -1,11 +1,15 @@
+import { TypeError } from './commons.js';
+
 /**
  * makeEvalFunction()
  * A safe version of the native eval function which relies on
- * the safety of safeEvaluate for confinement.
+ * the safety of safeEvaluate for confinement, unless noEval
+ * is specified (then a TypeError is thrown).
  *
- * @param {Function} safeEvaluate
+ * @param {Function} evaluator
+ * @param legacyHermesTaming
  */
-export const makeEvalFunction = safeEvaluate => {
+export const makeEvalFunction = (evaluator, legacyHermesTaming) => {
   // We use the concise method syntax to create an eval without a
   // [[Construct]] behavior (such that the invocation "new eval()" throws
   // TypeError: eval is not a constructor"), but which still accepts a
@@ -19,7 +23,30 @@ export const makeEvalFunction = safeEvaluate => {
         // rule. Track.
         return source;
       }
-      return safeEvaluate(source);
+      if (legacyHermesTaming === 'unsafe') {
+        throw TypeError(
+          `Legacy Hermes unsupported eval() called with string arguments cannot be tamed safe under legacyHermesTaming ${legacyHermesTaming}
+  See: https://github.com/facebook/hermes/issues/1056
+  See: https://github.com/endojs/endo/issues/1561
+Did you mean evalTaming: 'unsafeEval'?`,
+        );
+      }
+      // refactoring to try/catch...
+      // - error output still 'Uncaught'
+      // - SES_NO_EVAL no longer encountered first
+      // try {
+      //   safeEvaluate(source);
+      // } catch (e) {
+      //   // throw Error(e); // Uncaught Error: SyntaxError: 2:5:invalid statement encountered.
+      //   throw TypeError(
+      //     `legacy Hermes unsupported eval() called with string arguments cannot be tamed safe under legacyHermesTaming ${legacyHermesTaming}
+      // see: https://github.com/facebook/hermes/issues/1056
+      // see: https://github.com/endojs/endo/issues/1561
+      // did you mean evalTaming: 'unsafeEval'?`,
+      //   );
+      // }
+      // Disabling safeEvaluate is not enough, since returning the source string is not evaluating it.
+      return evaluator(source);
     },
   }.eval;
 
diff --git a/packages/ses/src/make-function-constructor.js b/packages/ses/src/make-function-constructor.js
@@ -12,9 +12,10 @@ const { Fail } = assert;
 /*
  * makeFunctionConstructor()
  * A safe version of the native Function which relies on
- * the safety of safeEvaluate for confinement.
+ * the safety of safeEvaluate for confinement, unless noEval
+ * is specified (then a TypeError is thrown).
  */
-export const makeFunctionConstructor = safeEvaluate => {
+export const makeFunctionConstructor = evaluator => {
   // Define an unused parameter to ensure Function.length === 1
   const newFunction = function Function(_body) {
     // Sanitize all parameters at the entry point.
@@ -54,7 +55,7 @@ export const makeFunctionConstructor = safeEvaluate => {
     // TODO: since we create an anonymous function, the 'this' value
     // isn't bound to the global object as per specs, but set as undefined.
     const src = `(function anonymous(${parameters}\n) {\n${bodyText}\n})`;
-    return safeEvaluate(src);
+    return evaluator(src);
   };
 
   defineProperties(newFunction, {
diff --git a/packages/ses/src/permits.js b/packages/ses/src/permits.js
@@ -1,7 +1,11 @@
 /* eslint-disable no-restricted-globals */
 /* eslint max-lines: 0 */
 
-import { arrayPush, arrayForEach } from './commons.js';
+import {
+  arrayPush,
+  arrayForEach,
+  getOwnPropertyDescriptor,
+} from './commons.js';
 
 /** @import {GenericErrorConstructor} from '../types.js' */
 
@@ -316,23 +320,37 @@ const accessor = {
   set: fn,
 };
 
+// TODO Remove this once we no longer support Hermes.
+// While all engines have a ThrowTypeError accessor for fields not permitted in strict mode,
+// some (Hermes 0.12) put that accessor in unexpected places.
+// We can't clean them up because they're non-configurable.
+// Therefore we're checking for identity with specCompliantThrowTypeError and dynamically adding permits for those.
+
 // eslint-disable-next-line func-names
-const strict = function () {
+const specCompliantThrowTypeError = (function () {
   'use strict';
-};
 
-// TODO Remove this once we no longer support the Hermes that needed this.
-arrayForEach(['caller', 'arguments'], prop => {
-  try {
-    strict[prop];
-  } catch (e) {
-    // https://github.com/facebook/hermes/blob/main/test/hermes/function-non-strict.js
-    if (e.message === 'Restricted in strict mode') {
-      // Fixed in Static Hermes: https://github.com/facebook/hermes/issues/1582
+  // eslint-disable-next-line prefer-rest-params
+  const desc = getOwnPropertyDescriptor(arguments, 'callee');
+  return desc && desc.get;
+})();
+if (specCompliantThrowTypeError) {
+  // eslint-disable-next-line func-names
+  const strict = function () {
+    'use strict';
+  };
+  arrayForEach(['caller', 'arguments'], prop => {
+    const desc = getOwnPropertyDescriptor(strict, prop);
+    if (
+      desc &&
+      desc.configurable === false &&
+      desc.get &&
+      desc.get === specCompliantThrowTypeError
+    ) {
       FunctionInstance[prop] = accessor;
     }
-  }
-});
+  });
+}
 
 export const isAccessorPermit = permit => {
   return permit === getter || permit === accessor;
diff --git a/packages/ses/types.d.ts b/packages/ses/types.d.ts
@@ -40,6 +40,13 @@ export interface RepairOptions {
   overrideTaming?: 'moderate' | 'min' | 'severe';
   overrideDebug?: Array<string>;
   domainTaming?: 'safe' | 'unsafe';
+  /**
+   * safe (default): do nothing.
+   *
+   * unsafe: skips direct-eval check and compartment.
+   *
+   */
+  legacyHermesTaming?: 'safe' | 'unsafe';
   /**
    * safe (default): do nothing.
    *
