feat(ses): tame Symbol so whitelist works

packages/ses/src/commons.js

@@ -33,6 +33,7 @@ export const {
   RegExp: FERAL_REG_EXP,
   Set,
   String,
+  Symbol,
   WeakMap,
   WeakSet,
 } = globalThis;

packages/ses/src/lockdown-shim.js

@@ -53,6 +53,7 @@ import { makeEnvironmentCaptor } from './environment-options.js';
 import { getAnonymousIntrinsics } from './get-anonymous-intrinsics.js';
 import { makeCompartmentConstructor } from './compartment-shim.js';
 import { tameHarden } from './tame-harden.js';
+import { tameSymbolConstructor } from './tame-symbol-constructor.js';
 
 /** @typedef {import('../types.js').LockdownOptions} LockdownOptions */
 
@@ -275,6 +276,7 @@ export const repairIntrinsics = (options = {}) => {
   addIntrinsics(tameErrorConstructor(errorTaming, stackFiltering));
   addIntrinsics(tameMathObject(mathTaming));
   addIntrinsics(tameRegExpConstructor(regExpTaming));
+  addIntrinsics(tameSymbolConstructor());
 
   addIntrinsics(getAnonymousIntrinsics());
 

packages/ses/src/tame-symbol-constructor.js

@@ -0,0 +1,65 @@
+import {
+  Symbol,
+  entries,
+  fromEntries,
+  getOwnPropertyDescriptors,
+  defineProperties,
+  arrayMap,
+} from './commons.js';
+
+/**
+ * This taming provides a tamed alternative to the original `Symbol` constructor
+ * that starts off identical, except that all its properties are "temporarily"
+ * configurable. The original `Symbol` constructor remains unmodified on
+ * the start compartment's global. The tamed alternative is used as the shared
+ * `Symbol` constructor on constructed compartments.
+ *
+ * Starting these properties as configurable assumes two succeeding phases of
+ * processing: A whitelisting phase, that
+ * removes all properties not on the whitelist (which requires them to be
+ * configurable) and a global hardening step that freezes all primordials,
+ * returning these properties to their expected non-configurable status.
+ *
+ * The ses shim is constructed to eventually enable vetted shims to run between
+ * repair and global hardening. However, such vetted shims would normally
+ * run in the start compartment, which continues to use the original unmodified
+ * `Symbol`, so they should not normally be affected by the temporary
+ * configurability of these properties.
+ *
+ * Note that the spec refers to the global `Symbol` function as the
+ * ["Symbol Constructor"](https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-symbol-constructor)
+ * even though it has a call behavior (can be called as a function) and does not
+ * not have a construct behavior (cannot be called with `new`). Accordingly,
+ * to tame it, we must replace it with a function without a construct
+ * behavior.
+ */
+export const tameSymbolConstructor = () => {
+  const OriginalSymbol = Symbol;
+  const SymbolPrototype = OriginalSymbol.prototype;
+
+  const SharedSymbol = {
+    Symbol(description) {
+      return OriginalSymbol(description);
+    },
+  }.Symbol;
+
+  defineProperties(SymbolPrototype, {
+    constructor: {
+      value: SharedSymbol,
+      // leave other `constructor` attributes as is
+    },
+  });
+
+  const originalDescsEntries = entries(
+    getOwnPropertyDescriptors(OriginalSymbol),
+  );
+  const descs = fromEntries(
+    arrayMap(originalDescsEntries, ([name, desc]) => [
+      name,
+      { ...desc, configurable: true },
+    ]),
+  );
+  defineProperties(SharedSymbol, descs);
+
+  return { '%SharedSymbol%': SharedSymbol };
+};

packages/ses/src/whitelist-intrinsics.js

@@ -47,6 +47,7 @@ import { whitelist, FunctionInstance, isAccessorPermit } from './whitelist.js';
 import {
   Map,
   String,
+  Symbol,
   TypeError,
   arrayFilter,
   arrayIncludes,
@@ -78,15 +79,14 @@ export default function whitelistIntrinsics(
 
   // These symbols are allowed as well-known symbols
   const wellKnownSymbolNames = new Map(
-    intrinsics.Symbol
+    Symbol
       ? arrayMap(
           arrayFilter(
-            entries(whitelist.Symbol),
+            entries(whitelist['%SharedSymbol%']),
             ([name, permit]) =>
-              permit === 'symbol' &&
-              typeof intrinsics.Symbol[name] === 'symbol',
+              permit === 'symbol' && typeof Symbol[name] === 'symbol',
           ),
-          ([name]) => [intrinsics.Symbol[name], `@@${name}`],
+          ([name]) => [Symbol[name], `@@${name}`],
         )
       : [],
   );

packages/ses/src/whitelist.js

@@ -67,7 +67,6 @@ export const universalPropertyNames = {
   ReferenceError: 'ReferenceError',
   Set: 'Set',
   String: 'String',
-  Symbol: 'Symbol',
   SyntaxError: 'SyntaxError',
   TypeError: 'TypeError',
   Uint8Array: 'Uint8Array',
@@ -109,6 +108,11 @@ export const initialGlobalPropertyNames = {
   Error: '%InitialError%',
   RegExp: '%InitialRegExp%',
 
+  // Omit `Symbol`, because we want the original to appear on the
+  // start compartment without passing through the whitelist mechanism, since
+  // we want to preserve all its properties, even if we never heard of them.
+  // Symbol: '%InitialSymbol%',
+
   // *** Other Properties of the Global Object
 
   Math: '%InitialMath%',
@@ -137,6 +141,7 @@ export const sharedGlobalPropertyNames = {
   Date: '%SharedDate%',
   Error: '%SharedError%',
   RegExp: '%SharedRegExp%',
+  Symbol: '%SharedSymbol%',
 
   // *** Other Properties of the Global Object
 
@@ -492,7 +497,7 @@ export const whitelist = {
     valueOf: fn,
   },
 
-  Symbol: {
+  '%SharedSymbol%': {
     // Properties of the Symbol Constructor
     '[[Proto]]': '%FunctionPrototype%',
     asyncDispose: 'symbol',
@@ -517,7 +522,7 @@ export const whitelist = {
 
   '%SymbolPrototype%': {
     // Properties of the Symbol Prototype Object
-    constructor: 'Symbol',
+    constructor: '%SharedSymbol%',
     description: getter,
     toString: fn,
     valueOf: fn,

packages/ses/test/test-get-global-intrinsics.js

@@ -50,7 +50,7 @@ test.skip('getGlobalIntrinsics', () => {
     'Set',
     // 'SharedArrayBuffer'  // removed on Jan 5, 2018
     'String',
-    'Symbol',
+    // 'Symbol', original left in place, but omitted from whitelist
     'SyntaxError',
     'TypeError',
     'Uint8Array',

packages/ses/test/test-tame-symbol-constructor.js

@@ -0,0 +1,39 @@
+import test from 'ava';
+import '../index.js';
+
+import {
+  defineProperty,
+  getOwnPropertyDescriptor as gopd,
+  isExtensible,
+  isFrozen,
+} from '../src/commons.js';
+
+defineProperty(Symbol, 'dummy', {
+  value: Symbol.for('faux well known symbol'),
+  writable: false,
+  enumerable: false,
+  configurable: false,
+});
+
+// Since %SharedSymbol%.dummy is not even mentioned on the whitelist,
+// this test should also print on the console:
+// > Removing intrinsics.%SharedSymbol%.dummy
+lockdown();
+
+test('Symbol cleaned by whitelist', t => {
+  t.true('dummy' in Symbol);
+  t.false(gopd(Symbol, 'iterator').configurable);
+  t.true(isExtensible(Symbol));
+  t.false(isFrozen(Symbol));
+  t.not(Symbol.constructor, Symbol);
+
+  const c = new Compartment();
+  const SharedSymbol = c.globalThis.Symbol;
+  t.is(Symbol.prototype, SharedSymbol.prototype);
+
+  t.false('dummy' in SharedSymbol);
+  t.false(gopd(SharedSymbol, 'iterator').configurable);
+  t.false(isExtensible(SharedSymbol));
+  t.true(isFrozen(SharedSymbol));
+  t.is(SharedSymbol.prototype.constructor, SharedSymbol);
+});

packages/ses/test/test-whitelist-intrinsics.js

@@ -7,7 +7,11 @@ if (!eval.toString().includes('native code')) {
   throw TypeError('Module "esm" enabled: aborting');
 }
 
-test('whitelistIntrinsics - Well-known symbols', t => {
+// This test as stated no longer makes sense, now that `whitelistIntrinsics`
+// decodes symbol strings using the real `globalThis.Symbol` rather than
+// `intrinsics.Symbol`. But I left this in as a `test.skip` in case anyone
+// wants to rescure some of the purpose of this test.
+test.skip('whitelistIntrinsics - Well-known symbols', t => {
   const SymbolIterator = Symbol('Symbol.iterator');
   const RogueSymbolIterator = Symbol('Symbol.iterator');
   const ArrayProto = { [RogueSymbolIterator]() {} };