refactor: prepare for non-trapping integrity trait

Author: erights

packages/captp/src/trap.js

@@ -1,5 +1,7 @@
 // Lifted mostly from `@endo/eventual-send/src/E.js`.
 
+const { freeze } = Object;
+
 /**
  * Default implementation of Trap for near objects.
  *
@@ -62,11 +64,21 @@ const TrapProxyHandler = (x, trapImpl) => {
  */
 export const makeTrap = trapImpl => {
   const Trap = x => {
+    /**
+     * `freeze` but not `harden` the proxy target so it remains trapping.
+     * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+     */
+    const target = freeze(() => {});
     const handler = TrapProxyHandler(x, trapImpl);
-    return harden(new Proxy(() => {}, handler));
+    return new Proxy(target, handler);
   };
 
   const makeTrapGetterProxy = x => {
+    /**
+     * `freeze` but not `harden` the proxy target so it remains trapping.
+     * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+     */
+    const target = freeze(Object.create(null));
     const handler = harden({
       ...baseFreezableProxyHandler,
       has(_target, _prop) {
@@ -77,7 +89,7 @@ export const makeTrap = trapImpl => {
         return trapImpl.get(x, prop);
       },
     });
-    return new Proxy(Object.create(null), handler);
+    return new Proxy(target, handler);
   };
   Trap.get = makeTrapGetterProxy;
 

packages/eventual-send/src/E.js

@@ -2,7 +2,7 @@ import { trackTurns } from './track-turns.js';
 import { makeMessageBreakpointTester } from './message-breakpoints.js';
 
 const { details: X, quote: q, Fail, error: makeError } = assert;
-const { assign, create } = Object;
+const { assign, create, freeze } = Object;
 
 /**
  * @import { HandledPromiseConstructor } from './types.js';
@@ -171,6 +171,16 @@ const makeEGetProxyHandler = (x, HandledPromise) =>
  * @param {HandledPromiseConstructor} HandledPromise
  */
 const makeE = HandledPromise => {
+  /**
+   * `freeze` but not `harden` the proxy target so it remains trapping.
+   * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+   */
+  const funcTarget = freeze(() => {});
+  /**
+   * `freeze` but not `harden` the proxy target so it remains trapping.
+   * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+   */
+  const objTarget = freeze(create(null));
   return harden(
     assign(
       /**
@@ -183,7 +193,7 @@ const makeE = HandledPromise => {
        * @returns {ECallableOrMethods<RemoteFunctions<T>>} method/function call proxy
        */
       // @ts-expect-error XXX typedef
-      x => harden(new Proxy(() => {}, makeEProxyHandler(x, HandledPromise))),
+      x => new Proxy(funcTarget, makeEProxyHandler(x, HandledPromise)),
       {
         /**
          * E.get(x) returns a proxy on which you can get arbitrary properties.
@@ -196,11 +206,8 @@ const makeE = HandledPromise => {
          * @returns {EGetters<LocalRecord<T>>} property get proxy
          * @readonly
          */
-        get: x =>
-          // @ts-expect-error XXX typedef
-          harden(
-            new Proxy(create(null), makeEGetProxyHandler(x, HandledPromise)),
-          ),
+        // @ts-expect-error XXX typedef
+        get: x => new Proxy(objTarget, makeEGetProxyHandler(x, HandledPromise)),
 
         /**
          * E.resolve(x) converts x to a handled promise. It is
@@ -224,9 +231,7 @@ const makeE = HandledPromise => {
          */
         sendOnly: x =>
           // @ts-expect-error XXX typedef
-          harden(
-            new Proxy(() => {}, makeESendOnlyProxyHandler(x, HandledPromise)),
-          ),
+          new Proxy(funcTarget, makeESendOnlyProxyHandler(x, HandledPromise)),
 
         /**
          * E.when(x, res, rej) is equivalent to

packages/eventual-send/src/handled-promise.js

@@ -309,6 +309,9 @@ export const makeHandledPromise = () => {
         if (proxyOpts) {
           const {
             handler: proxyHandler,
+            // The proxy target can be frozen but should not be hardened
+            // so it remains trapping.
+            // See https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
             target: proxyTarget,
             revokerCallback,
           } = proxyOpts;

packages/far/test/marshal-far-function.test.js

@@ -58,7 +58,7 @@ test('Data can contain far functions', t => {
   const arrow = Far('arrow', a => a + 1);
   t.is(passStyleOf(harden({ x: 8, foo: arrow })), 'copyRecord');
   const mightBeMethod = a => a + 1;
-  t.throws(() => passStyleOf(freeze({ x: 8, foo: mightBeMethod })), {
+  t.throws(() => passStyleOf(harden({ x: 8, foo: mightBeMethod })), {
     message: /Remotables with non-methods like "x" /,
   });
 });

packages/marshal/src/marshal-stringify.js

@@ -5,6 +5,8 @@ import { makeMarshal } from './marshal.js';
 
 /** @import {Passable} from '@endo/pass-style' */
 
+const { freeze } = Object;
+
 /** @type {import('./types.js').ConvertValToSlot<any>} */
 const doNotConvertValToSlot = val =>
   Fail`Marshal's stringify rejects presences and promises ${val}`;
@@ -23,7 +25,12 @@ const badArrayHandler = harden({
   },
 });
 
-const badArray = harden(new Proxy(harden([]), badArrayHandler));
+/**
+ * `freeze` but not `harden` the proxy target so it remains trapping.
+ * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+ */
+const arrayTarget = freeze(/** @type {any[]} */ ([]));
+const badArray = new Proxy(arrayTarget, badArrayHandler);
 
 const { serialize, unserialize } = makeMarshal(
   doNotConvertValToSlot,
@@ -48,7 +55,10 @@ harden(stringify);
  */
 const parse = str =>
   unserialize(
-    harden({
+    // `freeze` but not `harden` since the `badArray` proxy and its target
+    // must remain trapping.
+    // See https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+    freeze({
       body: str,
       slots: badArray,
     }),

packages/marshal/test/marshal-far-function.test.js

@@ -60,7 +60,7 @@ test('Data can contain far functions', t => {
   const arrow = Far('arrow', a => a + 1);
   t.is(passStyleOf(harden({ x: 8, foo: arrow })), 'copyRecord');
   const mightBeMethod = a => a + 1;
-  t.throws(() => passStyleOf(freeze({ x: 8, foo: mightBeMethod })), {
+  t.throws(() => passStyleOf(harden({ x: 8, foo: mightBeMethod })), {
     message: /Remotables with non-methods like "x" /,
   });
 });

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

@@ -13,7 +13,7 @@ const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
   global.harden
 );
 
-const { getPrototypeOf, defineProperty } = Object;
+const { getPrototypeOf, defineProperty, freeze } = Object;
 const { ownKeys } = Reflect;
 
 test('passStyleOf basic success cases', t => {
@@ -193,16 +193,22 @@ test('passStyleOf testing remotables', t => {
 
   const tagRecord1 = harden(makeTagishRecord('Alleged: manually constructed'));
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
-  const farObj1 = harden({
-    __proto__: tagRecord1,
-  });
+  const farObj1 = harden({ __proto__: tagRecord1 });
   t.is(passStyleOf(farObj1), 'remotable');
 
   const tagRecord2 = makeTagishRecord('Alleged: tagRecord not hardened');
-  /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
-  const farObj2 = Object.freeze({
-    __proto__: tagRecord2,
-  });
+  /**
+   * TODO In order to run this test before we have explicit support for a
+   * non-trapping integrity trait, we have to `freeze` here but not `harden`.
+   * However, once we do have that support, and `passStyleOf` checks that
+   * its argument is also non-trapping, we still need to avoid `harden`
+   * because that would also hardden `__proto__`. So we will need to
+   * explicitly make this non-trapping, which we cannot yet express.
+   * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+   *
+   * @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385
+   */
+  const farObj2 = freeze({ __proto__: tagRecord2 });
   if (harden.isFake) {
     t.is(passStyleOf(farObj2), 'remotable');
   } else {
@@ -212,39 +218,27 @@ test('passStyleOf testing remotables', t => {
     });
   }
 
-  const tagRecord3 = Object.freeze(
-    makeTagishRecord('Alleged: both manually frozen'),
-  );
+  const tagRecord3 = harden(makeTagishRecord('Alleged: both manually frozen'));
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
-  const farObj3 = Object.freeze({
-    __proto__: tagRecord3,
-  });
+  const farObj3 = harden({ __proto__: tagRecord3 });
   t.is(passStyleOf(farObj3), 'remotable');
 
   const tagRecord4 = harden(makeTagishRecord('Remotable'));
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
-  const farObj4 = harden({
-    __proto__: tagRecord4,
-  });
+  const farObj4 = harden({ __proto__: tagRecord4 });
   t.is(passStyleOf(farObj4), 'remotable');
 
   const tagRecord5 = harden(makeTagishRecord('Not alleging'));
-  const farObj5 = harden({
-    __proto__: tagRecord5,
-  });
+  const farObj5 = harden({ __proto__: tagRecord5 });
   t.throws(() => passStyleOf(farObj5), {
     message:
       /For now, iface "Not alleging" must be "Remotable" or begin with "Alleged: " or "DebugName: "; unimplemented/,
   });
 
   const tagRecord6 = harden(makeTagishRecord('Alleged: manually constructed'));
-  const farObjProto6 = harden({
-    __proto__: tagRecord6,
-  });
+  const farObjProto6 = harden({ __proto__: tagRecord6 });
   /** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
-  const farObj6 = harden({
-    __proto__: farObjProto6,
-  });
+  const farObj6 = harden({ __proto__: farObjProto6 });
   t.is(passStyleOf(farObj6), 'remotable', 'tagRecord grandproto is accepted');
 
   // Our current agoric-sdk plans for far classes are to create a class-like
@@ -323,12 +317,8 @@ test('passStyleOf testing remotables', t => {
   const fauxTagRecordB = harden(
     makeTagishRecord('Alleged: manually constructed', harden({})),
   );
-  const farObjProtoB = harden({
-    __proto__: fauxTagRecordB,
-  });
-  const farObjB = harden({
-    __proto__: farObjProtoB,
-  });
+  const farObjProtoB = harden({ __proto__: fauxTagRecordB });
+  const farObjB = harden({ __proto__: farObjProtoB });
   t.throws(() => passStyleOf(farObjB), {
     message:
       'cannot serialize Remotables with non-methods like "Symbol(passStyle)" in "[Alleged: manually constructed]"',
@@ -387,7 +377,16 @@ test('remotables - safety from the gibson042 attack', t => {
     },
   );
 
-  const makeInput = () => Object.freeze({ __proto__: mercurialProto });
+  /**
+   * TODO In order to run this test before we have explicit support for a
+   * non-trapping integrity trait, we have to `freeze` here but not `harden`.
+   * However, once we do have that support, and `passStyleOf` checks that
+   * its argument is also non-trapping, we still need to avoid `harden`
+   * because that would also hardden `__proto__`. So we will need to
+   * explicitly make this non-trapping, which we cannot yet express.
+   * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+   */
+  const makeInput = () => freeze({ __proto__: mercurialProto });
   const input1 = makeInput();
   const input2 = makeInput();
 

packages/ses/docs/preparing-for-stabilize.md

@@ -0,0 +1,24 @@
+# Preparing for the Non-trapping Integrity Trait
+
+The [Stabilize proposal](https://github.com/tc39/proposal-stabilize) is currently at stage 1 of the tc39 process. It proposes three distinct integrity traits whose current placeholder names are:
+- ***fixed***: would mitigate the return-override mistake by preventing objects with this trait from being stamped with new class-private-fields.
+- ***overridable***: would mitigate the assignment-override mistake by enabling non-writable properties inherited from an object with this trait to be overridden by property assignment on an inheriting object.
+- ***non-trapping***: would mitigate proxy-based reentrancy hazards by having a proxy whose target carries this trait never trap to its handler, but rather just perform the default action directly on this non-trapping target.
+
+Draft PR [feat(no-trapping-shim): ponyfill and shim for the no-trapping integrity trait #2673](https://github.com/endojs/endo/pull/2673) is a ponyfill and shim for this non-trapping integrity trait. The names it introduces are placeholders, since the bikeshedding process for these names has not yet concluded.
+
+Draft PR [feat(ses,pass-style): use non-trapping integrity trait for safety #2675](https://github.com/endojs/endo/pull/2675) uses this support for the non-trapping integity trait to mitigate reentrancy attacks from hardened objects, expecially passable copy-data objects like copyLists, copyRecords, and taggeds. To do so, it makes two fundamental changes:
+- Where `harden` made the object at every step frozen, that PR changes `harden` to also make those objects non-trapping.
+- Where `passStyleOf` checked that objects are frozen, that PR changes `passStyleOf` to also check that those objects are non-trapping.
+
+## How proxy code should prepare
+
+[#2673](https://github.com/endojs/endo/pull/2673) will *by default* produce proxies that refuse to be made non-trapping. An explicit handler trap (whose name is TBD) will need to be explicitly provided to make a proxy that allows itself to be made non-trapping. This is the right default, because proxies on frozen almost-empty objects can still have useful trap behavior for their `get`, `set`, `has`, and `apply` traps. Even on a frozen target
+- The `get`, `set`, and `has` traps applied to a non-own property name are still general traps that can have useful trapping behavior.
+- The `apply` trap can ignore the target's call behavior and just do its own thing.
+
+However, to prepare for these changes, we need to avoid hardening both such proxies and their targets. We need to avoid hardening their target because this will bypass the traps. We need to avoid hardening the proxy because such proxies will *by default* refuse to be made non-trapping, and thus refuse to be hardened.
+
+## How passable objects should prepare
+
+Although we think of `passStyleOf` as requiring its input to be hardened, `passStyleOf` instead checked that each relevant object is frozen. Manually freezing all objects reachable from a root object had been equivalent to hardening that root object. With these changes, even such manual transitive freezing will not make an object passable. To prepare for these changes, use `harden` explicitly instead.

packages/ses/src/commons.js

@@ -269,12 +269,6 @@ export const finalizationRegistryUnregister =
 export const getConstructorOf = fn =>
   reflectGet(getPrototypeOf(fn), 'constructor');
 
-/**
- * immutableObject
- * An immutable (frozen) empty object that is safe to share.
- */
-export const immutableObject = freeze(create(null));
-
 /**
  * isObject tests whether a value is an object.
  * Today, this is equivalent to:

packages/ses/src/sloppy-globals-scope-terminator.js

@@ -3,14 +3,20 @@ import {
   create,
   freeze,
   getOwnPropertyDescriptors,
-  immutableObject,
   reflectSet,
 } from './commons.js';
 import {
   strictScopeTerminatorHandler,
   alwaysThrowHandler,
 } from './strict-scope-terminator.js';
 
+/**
+ * `freeze` but not `harden` the proxy target so it remains trapping.
+ * Thus, it should not be shared outside this module.
+ * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+ */
+const onlyFrozenObject = freeze(create(null));
+
 /*
  * createSloppyGlobalsScopeTerminator()
  * strictScopeTerminatorHandler manages a scopeTerminator Proxy which serves as
@@ -45,7 +51,7 @@ export const createSloppyGlobalsScopeTerminator = globalObject => {
   );
 
   const sloppyGlobalsScopeTerminator = new Proxy(
-    immutableObject,
+    onlyFrozenObject,
     sloppyGlobalsScopeTerminatorHandler,
   );
 

packages/ses/src/strict-scope-terminator.js

@@ -7,12 +7,18 @@ import {
   freeze,
   getOwnPropertyDescriptors,
   globalThis,
-  immutableObject,
 } from './commons.js';
 import { assert } from './error/assert.js';
 
 const { Fail, quote: q } = assert;
 
+/**
+ * `freeze` but not `harden` the proxy target so it remains trapping.
+ * Thus, it should not be shared outside this module.
+ * @see https://github.com/endojs/endo/blob/master/packages/ses/docs/preparing-for-stabilize.md
+ */
+const onlyFrozenObject = freeze(create(null));
+
 /**
  * alwaysThrowHandler
  * This is an object that throws if any property is called. It's used as
@@ -21,7 +27,7 @@ const { Fail, quote: q } = assert;
  * create one and share it between all Proxy handlers.
  */
 export const alwaysThrowHandler = new Proxy(
-  immutableObject,
+  onlyFrozenObject,
   freeze({
     get(_shadow, prop) {
       Fail`Please report unexpected scope handler trap: ${q(String(prop))}`;
@@ -88,6 +94,6 @@ export const strictScopeTerminatorHandler = freeze(
 );
 
 export const strictScopeTerminator = new Proxy(
-  immutableObject,
+  onlyFrozenObject,
   strictScopeTerminatorHandler,
 );