From fd0660f156fb23bd87c4dee40d2d80c11bd61062 Mon Sep 17 00:00:00 2001
From: "Mark S. Miller" <erights@gmail.com>
Date: Mon, 4 Nov 2024 14:10:14 -0800
Subject: [PATCH] fix(ses): fix #2598 with cauterizeProperty reuse

---
 packages/ses/src/cauterize-property.js        | 51 +++++++++++++++++++
 packages/ses/src/intrinsics.js                | 11 +++-
 packages/ses/src/permits-intrinsics.js        | 34 ++-----------
 .../tolerate-empty-prototype-toplevel.test.js | 24 +++++++++
 .../ses/test/tolerate-empty-prototype.test.js |  3 ++
 5 files changed, 93 insertions(+), 30 deletions(-)
 create mode 100644 packages/ses/src/cauterize-property.js
 create mode 100644 packages/ses/test/tolerate-empty-prototype-toplevel.test.js

diff --git a/packages/ses/src/cauterize-property.js b/packages/ses/src/cauterize-property.js
new file mode 100644
index 0000000000..1bbf83e008
--- /dev/null
+++ b/packages/ses/src/cauterize-property.js
@@ -0,0 +1,51 @@
+import { objectHasOwnProperty } from './commons.js';
+
+/**
+ * @import {Reporter} from './reporting-types.js'
+ */
+
+/**
+ *
+ * @param {object} obj
+ * @param {PropertyKey} prop
+ * @param {boolean} known
+ * @param {string} subPath
+ * @param {Reporter} reporter
+ * @returns {void}
+ */
+export const cauterizeProperty = (
+  obj,
+  prop,
+  known,
+  subPath,
+  { warn, error },
+) => {
+  // Either the object lacks a permit or the object doesn't match the
+  // permit.
+  // If the permit is specifically false, not merely undefined,
+  // this is a property we expect to see because we know it exists in
+  // some environments and we have expressly decided to exclude it.
+  // Any other disallowed property is one we have not audited and we log
+  // that we are removing it so we know to look into it, as happens when
+  // the language evolves new features to existing intrinsics.
+  if (!known) {
+    warn(`Removing ${subPath}`);
+  }
+  try {
+    delete obj[prop];
+  } catch (err) {
+    if (objectHasOwnProperty(obj, prop)) {
+      if (typeof obj === 'function' && prop === 'prototype') {
+        obj.prototype = undefined;
+        if (obj.prototype === undefined) {
+          warn(`Tolerating undeletable ${subPath} === undefined`);
+          return;
+        }
+      }
+      error(`failed to delete ${subPath}`, err);
+    } else {
+      error(`deleting ${subPath} threw`, err);
+    }
+    throw err;
+  }
+};
diff --git a/packages/ses/src/intrinsics.js b/packages/ses/src/intrinsics.js
index 3393d64880..93a1efc55d 100644
--- a/packages/ses/src/intrinsics.js
+++ b/packages/ses/src/intrinsics.js
@@ -1,3 +1,4 @@
+import { cauterizeProperty } from './cauterize-property.js';
 import {
   TypeError,
   WeakSet,
@@ -100,7 +101,15 @@ export const makeIntrinsicsCollector = () => {
       }
       const namePrototype = permit.prototype;
       if (!namePrototype) {
-        throw TypeError(`${name}.prototype property not permitted`);
+        cauterizeProperty(
+          intrinsic,
+          'prototype',
+          false,
+          `${name}.prototype`,
+          console, // TODO should be a proper Reporter
+        );
+        // eslint-disable-next-line no-continue
+        continue;
       }
       if (
         typeof namePrototype !== 'string' ||
diff --git a/packages/ses/src/permits-intrinsics.js b/packages/ses/src/permits-intrinsics.js
index a629dae8ea..bb1743aea4 100644
--- a/packages/ses/src/permits-intrinsics.js
+++ b/packages/ses/src/permits-intrinsics.js
@@ -61,6 +61,7 @@ import {
   ownKeys,
   symbolKeyFor,
 } from './commons.js';
+import { cauterizeProperty } from './cauterize-property.js';
 
 /**
  * @import {Reporter} from './reporting-types.js'
@@ -279,35 +280,10 @@ export default function removeUnpermittedIntrinsics(
       const subPermit = getSubPermit(obj, permit, propString);
 
       if (!subPermit || !isAllowedProperty(subPath, obj, prop, subPermit)) {
-        // Either the object lacks a permit or the object doesn't match the
-        // permit.
-        // If the permit is specifically false, not merely undefined,
-        // this is a property we expect to see because we know it exists in
-        // some environments and we have expressly decided to exclude it.
-        // Any other disallowed property is one we have not audited and we log
-        // that we are removing it so we know to look into it, as happens when
-        // the language evolves new features to existing intrinsics.
-        if (subPermit !== false) {
-          warn(`Removing ${subPath}`);
-        }
-        try {
-          delete obj[prop];
-        } catch (err) {
-          if (prop in obj) {
-            if (typeof obj === 'function' && prop === 'prototype') {
-              obj.prototype = undefined;
-              if (obj.prototype === undefined) {
-                warn(`Tolerating undeletable ${subPath} === undefined`);
-                // eslint-disable-next-line no-continue
-                continue;
-              }
-            }
-            error(`failed to delete ${subPath}`, err);
-          } else {
-            error(`deleting ${subPath} threw`, err);
-          }
-          throw err;
-        }
+        cauterizeProperty(obj, prop, subPermit === false, subPath, {
+          warn,
+          error,
+        });
       }
     }
   }
diff --git a/packages/ses/test/tolerate-empty-prototype-toplevel.test.js b/packages/ses/test/tolerate-empty-prototype-toplevel.test.js
new file mode 100644
index 0000000000..34582a3777
--- /dev/null
+++ b/packages/ses/test/tolerate-empty-prototype-toplevel.test.js
@@ -0,0 +1,24 @@
+/* global globalThis */
+import test from 'ava';
+import '../index.js';
+
+// See https://github.com/zloirock/core-js/issues/1092
+// See https://github.com/endojs/endo/issues/2598
+const originalEscape = globalThis.escape;
+globalThis.escape = function escape(...args) {
+  return Reflect.apply(originalEscape, this, args);
+};
+
+lockdown();
+
+test('tolerate empty escape.prototype', t => {
+  t.is(globalThis.escape, escape);
+  t.assert('prototype' in escape);
+  t.is(escape.prototype, undefined);
+  t.deepEqual(Object.getOwnPropertyDescriptor(escape, 'prototype'), {
+    value: undefined,
+    writable: !!harden.isFake,
+    enumerable: false,
+    configurable: false,
+  });
+});
diff --git a/packages/ses/test/tolerate-empty-prototype.test.js b/packages/ses/test/tolerate-empty-prototype.test.js
index 5ad78e018a..055f60340b 100644
--- a/packages/ses/test/tolerate-empty-prototype.test.js
+++ b/packages/ses/test/tolerate-empty-prototype.test.js
@@ -2,6 +2,9 @@ import test from 'ava';
 import '../index.js';
 
 // See https://github.com/zloirock/core-js/issues/1092
+// Does not detect https://github.com/endojs/endo/issues/2598 because
+// `push` is not toplevel.
+// See tolerate-empty-prototype-toplevel.test.js
 const originalPush = Array.prototype.push;
 // eslint-disable-next-line no-extend-native
 Array.prototype.push = function push(...args) {