fix: passStyleOf full input validation
MODIFY
packages/marshal/src/helpers/passStyle-helpers.js
packages/marshal/src/helpers/passStyle-helpers.js
@@ -9,6 +9,7 @@ const { details: X, quote: q } = assert;
const {
getOwnPropertyDescriptor,
hasOwnProperty: objectHasOwnProperty,
+ isFrozen,
} = Object;
const { apply } = Reflect;
const { isArray } = Array;
@@ -111,14 +112,14 @@ harden(getTag);
export const checkTagRecord = (tagRecord, passStyle, check = x => x) => {
return (
check(
- typeof tagRecord === 'object',
+ typeof tagRecord === 'object' && tagRecord !== null,
X`A non-object cannot be a tagRecord: ${tagRecord}`,
) &&
+ check(isFrozen(tagRecord), X`A tagRecord must be frozen: ${tagRecord}`) &&
check(
!isArray(tagRecord),
X`An array cannot be a tagRecords: ${tagRecord}`,
) &&
- check(tagRecord !== null, X`null cannot be a tagRecord`) &&
checkNormalProperty(tagRecord, PASS_STYLE, 'symbol', false, check) &&
check(
tagRecord[PASS_STYLE] === passStyle,

ADD
packages/marshal/src/helpers/safe-promise.js
+// @ts-check
+
+/// <reference types="ses"/>
+
+import { isPromise } from '@endo/promise-kit';
+import { assertChecker } from './passStyle-helpers.js';
+
+/** @typedef {import('../types.js').Checker} Checker */
+
+const { details: X, quote: q } = assert;
+const { isFrozen, getPrototypeOf } = Object;
+const { ownKeys } = Reflect;
+
+/**
+ * Under Hardened JS a promise is "safe" if its `then` method can be called
+ * synchronously without giving the promise an opportunity for a
+ * reentrancy attack during that call.
+ *
+ * https://github.com/Agoric/agoric-sdk/issues/9
+ * raises the issue of testing that a specimen is a safe promise
+ * such that the test also does not give the specimen a
+ * reentrancy opportunity. That is well beyond the ambition here.
+ * TODO Though if we figure out a nice solution, it might be good to
+ * use it here as well.
+ *
+ * @param {unknown} pr The value to examine
+ * @param {Checker} [check]
+ * @returns {pr is Promise} Whether it is a safe promise
+ */
+const checkSafePromise = (pr, check = x => x) => {
+ let keys;
+ return (
+ check(isFrozen(pr), X`${pr} - Must be frozen`) &&
+ check(isPromise(pr), X`${pr} - Must be a promise`) &&
+ check(
+ getPrototypeOf(pr) === Promise.prototype,
+ X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
+ ) &&
+ check(
+ // Suppressing prettier for the following line because it wants to
+ // remove the "extra" parens around `pr`. However, these parens are
+ // required for the TypeScript case syntax. We know this case is safe
+ // because we only get here if `ifPromise(pr)` already passed.
+ // eslint-disable-next-line prettier/prettier
+ (keys = ownKeys(/** @type {Promise} pr */(pr))).length === 0,
+ X`{pr} - Must not have any own properties: ${q(keys)}`,
+ )
+ );
+};
+harden(checkSafePromise);
+
+/**
+ * Determine if the argument is a Promise.
+ *
+ * @param {unknown} pr The value to examine
+ * @returns {pr is Promise} Whether it is a promise
+ */
+export const isSafePromise = pr => checkSafePromise(pr);
+harden(isSafePromise);
+
+export const assertSafePromise = pr => checkSafePromise(pr, assertChecker);

MODIFY
packages/marshal/src/passStyleOf.js
packages/marshal/src/passStyleOf.js
@@ -12,6 +12,7 @@ import { RemotableHelper } from './helpers/remotable.js';
import { ErrorHelper } from './helpers/error.js';
import { assertPassableSymbol } from './helpers/symbol.js';
+import { assertSafePromise } from './helpers/safe-promise.js';
/** @typedef {import('./helpers/internal-types.js').PassStyleHelper} PassStyleHelper */
/** @typedef {import('./types.js').Passable} Passable */
@@ -149,6 +150,7 @@ const makePassStyleOf = passStyleHelpers => {
X`Cannot pass non-frozen objects like ${inner}. Use harden()`,
);
if (isPromise(inner)) {
+ assertSafePromise(inner);
return 'promise';
}
assert(

ADD
packages/marshal/test/test-passStyleOf.js
+import { test } from './prepare-test-env-ava.js';
+import { passStyleOf } from '../src/passStyleOf.js';
+import { Far } from '../src/make-far.js';
+import { makeTagged } from '../src/makeTagged.js';
+import { PASS_STYLE } from '../src/helpers/passStyle-helpers.js';
+
+test('passStyleOf basic success cases', t => {
+ // Test in same order as `passStyleOf` for easier maintenance.
+ // Remotables tested separately below.
+ t.is(passStyleOf(undefined), 'undefined');
+ t.is(passStyleOf('foo'), 'string');
+ t.is(passStyleOf(true), 'boolean');
+ t.is(passStyleOf(33), 'number');
+ t.is(passStyleOf(33n), 'bigint');
+ t.is(passStyleOf(Symbol.for('foo')), 'symbol');
+ t.is(passStyleOf(Symbol.iterator), 'symbol');
+ t.is(passStyleOf(null), 'null');
+ t.is(passStyleOf(harden(Promise.resolve(null))), 'promise');
+ t.is(passStyleOf(harden([3, 4])), 'copyArray');
+ t.is(passStyleOf(harden({ foo: 3 })), 'copyRecord');
+ t.is(passStyleOf(harden({ then: 'non-function then ok' })), 'copyRecord');
+ t.is(passStyleOf(makeTagged('unknown', undefined)), 'tagged');
+ t.is(passStyleOf(harden(Error('ok'))), 'error');
+});
+
+test('some passStyleOf rejections', t => {
+ t.throws(() => passStyleOf(Symbol('unique')), {
+ message: /Only registered symbols or well-known symbols are passable: "\[Symbol\(unique\)\]"/,
+ });
+ t.throws(() => passStyleOf({}), {
+ message: /Cannot pass non-frozen objects like {}. Use harden\(\)/,
+ });
+
+ const prbad1 = Promise.resolve();
+ Object.setPrototypeOf(prbad1, { __proto__: Promise.prototype });
+ harden(prbad1);
+ t.throws(() => passStyleOf(prbad1), {
+ message: /"\[Promise\]" - Must inherit from Promise.prototype: "\[Promise\]"/,
+ });
+
+ const prbad2 = Promise.resolve();
+ prbad2.extra = 'unexpected own property';
+ harden(prbad2);
+ t.throws(() => passStyleOf(prbad2), {
+ message: /{pr} - Must not have any own properties: \["extra"\]/,
+ });
+
+ const prbad3 = Promise.resolve();
+ Object.defineProperty(prbad3, 'then', { value: () => 'bad then' });
+ harden(prbad3);
+ t.throws(() => passStyleOf(prbad3), {
+ message: /{pr} - Must not have any own properties: \["then"\]/,
+ });
+
+ const thenable1 = harden({ then: () => 'thenable' });
+ t.throws(() => passStyleOf(thenable1), {
+ message: /Cannot pass non-promise thenables/,
+ });
+
+ const thenable2 = Far('remote thenable', { then: () => 'thenable' });
+ t.throws(() => passStyleOf(thenable2), {
+ message: /Cannot pass non-promise thenables/,
+ });
+});
+
+test('passStyleOf testing remotables', t => {
+ t.is(passStyleOf(Far('foo', {})), 'remotable');
+ t.is(passStyleOf(Far('foo', () => 'far function')), 'remotable');
+
+ const tagRecord1 = Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Alleged: manually constructed' },
+ });
+ const farObj1 = harden({
+ __proto__: tagRecord1,
+ });
+ t.is(passStyleOf(farObj1), 'remotable');
+
+ const tagRecord2 = Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Alleged: tagRecord not hardened' },
+ });
+ const farObj2 = Object.freeze({
+ __proto__: tagRecord2,
+ });
+ t.throws(() => passStyleOf(farObj2), {
+ message: /A tagRecord must be frozen: "\[Alleged: tagRecord not hardened\]"/,
+ });
+
+ const tagRecord3 = Object.freeze(
+ Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Alleged: both manually frozen' },
+ }),
+ );
+ const farObj3 = Object.freeze({
+ __proto__: tagRecord3,
+ });
+ t.is(passStyleOf(farObj3), 'remotable');
+
+ const tagRecord4 = Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Remotable' },
+ });
+ const farObj4 = harden({
+ __proto__: tagRecord4,
+ });
+ t.is(passStyleOf(farObj4), 'remotable');
+
+ const tagRecord5 = Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Not alleging' },
+ });
+ const farObj5 = harden({
+ __proto__: tagRecord5,
+ });
+ t.throws(() => passStyleOf(farObj5), {
+ message: /For now, iface "Not alleging" must be "Remotable" or begin with "Alleged: "; unimplemented/,
+ });
+
+ // We need this to succeed to enable far classes
+ const tagRecord6 = Object.create(Object.prototype, {
+ [PASS_STYLE]: { value: 'remotable' },
+ [Symbol.toStringTag]: { value: 'Alleged: manually constructed' },
+ });
+ const farObjProto6 = harden({
+ __proto__: tagRecord6,
+ });
+ const farObj6 = harden({
+ __proto__: farObjProto6,
+ });
+ t.throws(() => passStyleOf(farObj6), {
+ message: /"\[Symbol\(passStyle\)\]" property expected: "\[Alleged: manually constructed\]"/,
+ });
+});

fix: passStyleOf full input validation (#1250)