fix(ses): harden hacks v8 stack own accessor problem

packages/pass-style/test/test-passStyleOf.js

@@ -416,12 +416,11 @@ test('Unexpected stack on errors', t => {
 
   const carrierStack = {};
   err.stack = carrierStack;
-  Object.freeze(err);
+  harden(err);
 
   t.throws(() => passStyleOf(err), {
     message: 'Passable Error "stack" own property must be a string: {}',
   });
-  err.stack.foo = 42;
 });
 
 test('Allow toStringTag overrides', t => {

packages/ses/src/commons.js

@@ -294,3 +294,58 @@ export const noEvalEvaluate = () => {
   // See https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_NO_EVAL.md
   throw TypeError('Cannot eval with evalTaming set to "noEval" (SES_NO_EVAL)');
 };
+
+// ////////////////// FERAL_STACK_GETTER FERAL_STACK_SETTER ////////////////////
+
+const er1StackDesc = getOwnPropertyDescriptor(Error('er1'), 'stack');
+const er2StackDesc = getOwnPropertyDescriptor(TypeError('er2'), 'stack');
+
+let feralStackGetter;
+let feralStackSetter;
+if (er1StackDesc && er2StackDesc && er1StackDesc.get) {
+  // We should only encounter this case on v8 because of its problematic
+  // error own stack accessor behavior.
+  // Note that FF/SpiderMonkey, Moddable/XS, and the error stack proposal
+  // all inherit a stack accessor property from Error.prototype, which is
+  // great. That case needs no heroics to secure.
+  if (
+    // In the v8 case as we understand it, all errors have an own stack
+    // accessor property, but within the same realm, all these accessor
+    // properties have the same getter and have the same setter.
+    // This is therefore the case that we repair.
+    typeof er1StackDesc.get === 'function' &&
+    er1StackDesc.get === er2StackDesc.get &&
+    typeof er1StackDesc.set === 'function' &&
+    er1StackDesc.set === er2StackDesc.set
+  ) {
+    // Otherwise, we have own stack accessor properties that are outside
+    // our expectations, that therefore need to be understood better
+    // before we know how to repair them.
+    feralStackGetter = er1StackDesc.get;
+    feralStackSetter = er1StackDesc.set;
+  } else {
+    throw TypeError('Error own stack accessor functions not as expected');
+  }
+}
+
+/**
+ * If on a v8 with the problematic error own stack accessor behavior,
+ * `FERAL_STACK_GETTER` will be the shared getter of all those accessors
+ * and `FERAL_STACK_SETTER` will be the shared setter. On any platform
+ * without this problem, `FERAL_STACK_GETTER` and `FERAL_STACK_SETTER` are
+ * both `undefined`.
+ *
+ * @type {(() => any) | undefined}
+ */
+export const FERAL_STACK_GETTER = feralStackGetter;
+
+/**
+ * If on a v8 with the problematic error own stack accessor behavior,
+ * `FERAL_STACK_GETTER` will be the shared getter of all those accessors
+ * and `FERAL_STACK_SETTER` will be the shared setter. On any platform
+ * without this problem, `FERAL_STACK_GETTER` and `FERAL_STACK_SETTER` are
+ * both `undefined`.
+ *
+ * @type {((newValue: any) => void) | undefined}
+ */
+export const FERAL_STACK_SETTER = feralStackSetter;

packages/ses/src/make-hardener.js

@@ -49,6 +49,8 @@ import {
   weakmapSet,
   weaksetAdd,
   weaksetHas,
+  FERAL_STACK_GETTER,
+  isError,
 } from './commons.js';
 import { assert } from './error/assert.js';
 
@@ -174,7 +176,7 @@ export const makeHardener = () => {
       /**
        * @param {any} obj
        */
-      function freezeAndTraverse(obj) {
+      const baseFreezeAndTraverse = obj => {
         // Now freeze the object to ensure reactive
         // objects such as proxies won't add properties
         // during traversal, before they get frozen.
@@ -218,21 +220,53 @@ export const makeHardener = () => {
             enqueue(desc.set, `${pathname}(set)`);
           }
         });
-      }
+      };
+
+      const freezeAndTraverse =
+        FERAL_STACK_GETTER === undefined
+          ? // On platforms without v8's error own stack accessor problem,
+            // don't pay for any extra overhead.
+            baseFreezeAndTraverse
+          : obj => {
+              if (isError(obj)) {
+                // Only pay the overhead if it first passes this cheap isError
+                // check. Otherwise, it will be unrepaired, but won't be judged
+                // to be a passable error anyway, so will not be unsafe.
+                const stackDesc = getOwnPropertyDescriptor(obj, 'stack');
+                if (
+                  stackDesc &&
+                  stackDesc.get === FERAL_STACK_GETTER &&
+                  stackDesc.configurable
+                ) {
+                  // Can only repair if it is configurable. Otherwise, leave
+                  // unrepaired, in which case it will not be judged passable,
+                  // avoiding a safety problem.
+                  defineProperty(obj, 'stack', {
+                    // NOTE: Calls getter during harden, which seems dangerous.
+                    // But we're only calling the problematic getter whose
+                    // hazards we think we understand.
+                    // @ts-expect-error TS should know FERAL_STACK_GETTER
+                    // cannot be `undefined` here.
+                    value: apply(FERAL_STACK_GETTER, obj, []),
+                  });
+                }
+              }
+              return baseFreezeAndTraverse(obj);
+            };
 
-      function dequeue() {
+      const dequeue = () => {
         // New values added before forEach() has finished will be visited.
         setForEach(toFreeze, freezeAndTraverse);
-      }
+      };
 
       /** @param {any} value */
-      function markHardened(value) {
+      const markHardened = value => {
         weaksetAdd(hardened, value);
-      }
+      };
 
-      function commit() {
+      const commit = () => {
         setForEach(toFreeze, markHardened);
-      }
+      };
 
       enqueue(root);
       dequeue();