Repository: endojs/endo
Pull request #2383 (2024-07-26 18:29:14)

feat: add regenerator runtime taming
Closes: #621
Refs: #1950

## Description

regenerator-runtime is a widely used package in the ecosystem. It is used to support generators and async functions transpiled to ES5.

This PR adds an option `legacyRegeneratorRuntimeTaming` to fix `regenerator-runtime` from 0.10.5 to 0.13.7. Although the newer version of the regenerator runtime package is compatible with lockdown, some libraries bundle old (hence "legacy") regenerator runtime in their code and it's not practical to get them all to upgrade.

- `legacyRegeneratorRuntimeTaming: 'safe'` do nothing.
- `legacyRegeneratorRuntimeTaming: 'unsafe-ignore'` turns `Iterator.prototype[@@iterator]` to a funky accessor that drops all assignments to it.

Note: `regenerator-runtime` is doing this:

```js
Gp[iteratorSymbol] = function () {
    return this;
}
```
which is effectively
```js
IteratorPrototype[Symbol.iterator] = function () {
    return this;
}
```

### Security Considerations

The replacement function from legacy regenerator runtime is the same as the native code, so it is "safe" to drop this assignment, in the sense that it does not cause any bad effects. However, this option drops the assignment by dropping any assignment to `IteratorPrototype[Symbol.iterator]`, since we have no practical way to ensure that the assignment it drops is exactly the one above. Thus, this option is not actual safe since it causes any other such assignment to be ignored silently. This echoes the unsafety of ES3 and of sloppy mode, where failed assignments were silently ignored. Such behavior is unsafe because it allows control flow to proceed into code that assumes the assignment succeeded. That's why ES5 strict mode changed failed assignments to throw. 

To emphasize the hazard, we have named this setting of the option `'unsafe-ignore'`.

### Scaling Considerations

Nothing

### Documentation Considerations

If you're hitting problems with an old version of regenerator-runtime (or any package that bundles it), you might need this.

### Testing Considerations

Tests added for the specific effect of this PR. However, to avoid introducing even a devDependency on a legacy version of regenerator runtime, no automated test has been added to test that compatibility. Instead, this PR can be tested like this:

```js
import './lockdown.umd.js'
lockdown({ legacyRegeneratorRuntimeTaming: 'unsafe-ignore', errorTaming: 'unsafe', consoleTaming: 'unsafe' })
const script = document.createElement('script')
script.src = 'https://cdn.jsdelivr.net/npm/regenerator-runtime@0.13.7/runtime.js'
document.head.appendChild(script)
```

### Compatibility Considerations

Note: Some version of `regenerator-runtime` requires to be run in the sloppy mode. Thus, these are incompat with the ses-shim independent of this option.

### Upgrade Considerations

No

> Update `NEWS.md` for user-facing changes.

TODO.

Jack-Works: hi, @erights I have updated the option name with a `legacy` prefix and changed `none` to `safe`, and `safe` to `unsafe-ignore`. I also changed how the setter behaves. I'd like to add a test after the design is OK.
Jack-Works: > Hi Jack, everything here looks good to me (modulo minor suggestions). Once there's a test case, I'll be happy to approve. Thanks!

Hi! I added tests!
Jack-Works: thanks for the review! I think I fixed everything you have mentioned.
erights: Hi @Jack-Works , I've LGTMed and am about to squash-and-merge. But before I do, I'm going to make some minor edits to the PR comment, which will then also be in the commit comment.

Thanks!
erights: Hi @Jack-Works , I notice you wrote

> > Update NEWS.md for user-facing changes.
>
> TODO.

Quite right. This reminds me that https://github.com/endojs/endo/blob/master/packages/ses/docs/lockdown.md will also need updating. I'm going to go ahead and merge this without either, but please do take a look at these. (attn @kriskowal )

Patch:
diff --git a/packages/ses/src/commons.js b/packages/ses/src/commons.js
index 5028a7f57..7b8c2ce3b 100644
--- a/packages/ses/src/commons.js
+++ b/packages/ses/src/commons.js
@@ -137,6 +137,10 @@ export const { prototype: generatorPrototype } = getPrototypeOf(
   // eslint-disable-next-line no-empty-function, func-names
   function* () {},
 );
+export const iteratorPrototype = getPrototypeOf(
+  // eslint-disable-next-line @endo/no-polymorphic-call
+  getPrototypeOf(arrayPrototype.values()),
+);
 
 export const typedArrayPrototype = getPrototypeOf(Uint8Array.prototype);
 
diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js
index 3a2e9c492..23bf238b6 100644
--- a/packages/ses/src/lockdown.js
+++ b/packages/ses/src/lockdown.js
@@ -55,6 +55,7 @@ import { makeCompartmentConstructor } from './compartment.js';
 import { tameHarden } from './tame-harden.js';
 import { tameSymbolConstructor } from './tame-symbol-constructor.js';
 import { tameFauxDataProperties } from './tame-faux-data-properties.js';
+import { tameRegeneratorRuntime } from './tame-regenerator-runtime.js';
 
 /** @import {LockdownOptions} from '../types.js' */
 
@@ -180,12 +181,20 @@ export const repairIntrinsics = (options = {}) => {
       /** @param {string} debugName */
       debugName => debugName !== '',
     ),
+    legacyRegeneratorRuntimeTaming = getenv(
+      'LOCKDOWN_LEGACY_REGENERATOR_RUNTIME_TAMING',
+      'safe',
+    ),
     __hardenTaming__ = getenv('LOCKDOWN_HARDEN_TAMING', 'safe'),
     dateTaming = 'safe', // deprecated
     mathTaming = 'safe', // deprecated
     ...extraOptions
   } = options;
 
+  legacyRegeneratorRuntimeTaming === 'safe' ||
+    legacyRegeneratorRuntimeTaming === 'unsafe-ignore' ||
+    Fail`lockdown(): non supported option legacyRegeneratorRuntimeTaming: ${q(legacyRegeneratorRuntimeTaming)}`;
+
   evalTaming === 'unsafeEval' ||
     evalTaming === 'safeEval' ||
     evalTaming === 'noEval' ||
@@ -412,6 +421,9 @@ export const repairIntrinsics = (options = {}) => {
     // clear yet which is better.
     // @ts-ignore enablePropertyOverrides does its own input validation
     enablePropertyOverrides(intrinsics, overrideTaming, overrideDebug);
+    if (legacyRegeneratorRuntimeTaming === 'unsafe-ignore') {
+      tameRegeneratorRuntime();
+    }
 
     // Finally register and optionally freeze all the intrinsics. This
     // must be the operation that modifies the intrinsics.
diff --git a/packages/ses/src/tame-regenerator-runtime.js b/packages/ses/src/tame-regenerator-runtime.js
new file mode 100644
index 000000000..0ea6c1c96
--- /dev/null
+++ b/packages/ses/src/tame-regenerator-runtime.js
@@ -0,0 +1,29 @@
+import {
+  defineProperty,
+  iteratorPrototype,
+  iteratorSymbol,
+  objectHasOwnProperty,
+} from './commons.js';
+
+export const tameRegeneratorRuntime = () => {
+  const iter = iteratorPrototype[iteratorSymbol];
+  defineProperty(iteratorPrototype, iteratorSymbol, {
+    configurable: true,
+    get() {
+      return iter;
+    },
+    set(value) {
+      // ignore the assignment on IteratorPrototype
+      if (this === iteratorPrototype) return;
+      if (objectHasOwnProperty(this, iteratorSymbol)) {
+        this[iteratorSymbol] = value;
+      }
+      defineProperty(this, iteratorSymbol, {
+        value,
+        writable: true,
+        enumerable: true,
+        configurable: true,
+      });
+    },
+  });
+};
diff --git a/packages/ses/types.d.ts b/packages/ses/types.d.ts
index ae342bcca..1cd2f5770 100644
--- a/packages/ses/types.d.ts
+++ b/packages/ses/types.d.ts
@@ -33,6 +33,12 @@ export interface RepairOptions {
   overrideTaming?: 'moderate' | 'min' | 'severe';
   overrideDebug?: Array<string>;
   domainTaming?: 'safe' | 'unsafe';
+  /**
+   * safe (default): do nothing.
+   *
+   * unsafe-ignore: make %IteratorPrototype%[@@iterator] to a funky accessor which ignores all assignments.
+   */
+  legacyRegeneratorRuntimeTaming?: 'safe' | 'unsafe-ignore';
   __hardenTaming__?: 'safe' | 'unsafe';
 }
 


Test patch:
diff --git a/packages/ses/test/tame-legacy-regenerator-helper.test.js b/packages/ses/test/tame-legacy-regenerator-helper.test.js
new file mode 100644
index 000000000..27dc07ffa
--- /dev/null
+++ b/packages/ses/test/tame-legacy-regenerator-helper.test.js
@@ -0,0 +1,45 @@
+import test from 'ava';
+import '../index.js';
+
+lockdown({ legacyRegeneratorRuntimeTaming: 'unsafe-ignore' });
+
+test('lockdown Iterator.prototype[@@iterator] is tamed', t => {
+  const IteratorProto = Object.getPrototypeOf(
+    Object.getPrototypeOf([].values()),
+  );
+  const desc = Object.getOwnPropertyDescriptor(IteratorProto, Symbol.iterator);
+  if (!desc || !desc.get || !desc.set) throw new Error('unreachable');
+  t.is(desc.configurable || desc.enumerable, false);
+  t.is(desc.value, undefined);
+
+  const { get } = desc;
+
+  const child = Object.create(IteratorProto);
+  child[Symbol.iterator] = 'foo'; // override test
+  t.is(child[Symbol.iterator], 'foo');
+
+  const native = get();
+  IteratorProto[Symbol.iterator] = function f() {
+    return this;
+  };
+  t.is(get(), native);
+  t.is(
+    Function.prototype.toString.call(native),
+    'function [Symbol.iterator]() { [native code] }',
+  );
+});
+
+test('lockdown Iterator.prototype[@@iterator] is tamed in Compartments', t => {
+  const c = new Compartment();
+  const compartmentIteratorProto = Object.getPrototypeOf(
+    Object.getPrototypeOf(c.globalThis.Array().values()),
+  );
+  t.is(
+    compartmentIteratorProto,
+    Object.getPrototypeOf(Object.getPrototypeOf([].values())),
+  );
+  const parentFunction = /** @type {any} */ (
+    Object.getOwnPropertyDescriptor(compartmentIteratorProto, Symbol.iterator)
+  ).get.constructor;
+  t.throws(() => Reflect.construct(parentFunction, ['return globalThis']));
+});