fix(marshal)!: Reject null as a substitute for Object.prototype
Fixes
MODIFY
packages/marshal/src/helpers/copyRecord.js
packages/marshal/src/helpers/copyRecord.js
@@ -25,9 +25,11 @@ export const CopyRecordHelper = harden({
canBeValid: (candidate, check) => {
const reject = !!check && (details => check(false, details));
- const proto = getPrototypeOf(candidate);
- if (proto !== objectPrototype && proto !== null) {
- return reject && reject(X`Unexpected prototype for: ${candidate}`);
+ if (getPrototypeOf(candidate) !== objectPrototype) {
+ return (
+ (reject &&
+ reject(X`Records must inherit from Object.prototype: ${candidate}`))
+ );
}
const descs = getOwnPropertyDescriptors(candidate);
const descKeys = ownKeys(descs);

MODIFY
packages/marshal/src/helpers/passStyle-helpers.js
packages/marshal/src/helpers/passStyle-helpers.js
const { details: X, quote: q } = assert;
const { isArray } = Array;
+const { prototype: functionPrototype } = Function;
const {
getOwnPropertyDescriptor,
getPrototypeOf,
hasOwnProperty: objectHasOwnProperty,
isFrozen,
+ prototype: objectPrototype,
} = Object;
const { apply } = Reflect;
const { toStringTag: toStringTagSymbol } = Symbol;
@@ -125,13 +127,27 @@ harden(checkNormalProperty);
export const getTag = tagRecord => tagRecord[Symbol.toStringTag];
harden(getTag);
+export const checkPassStyle = (obj, expectedPassStyle, check) => {
+ const actual = obj[PASS_STYLE];
+ return (
+ actual === expectedPassStyle ||
+ (!!check &&
+ check(
+ false,
+ X`Expected ${q(expectedPassStyle)}, not ${q(actual)}: ${obj}`,
+ ))
+ );
+};
+harden(checkPassStyle);
+
+const makeCheckTagRecord = checkProto => {
/**
* @param {{ [PASS_STYLE]: string }} tagRecord
* @param {PassStyle} passStyle
* @param {Checker} [check]
* @returns {boolean}
*/
-export const checkTagRecord = (tagRecord, passStyle, check) => {
+ const checkTagRecord = (tagRecord, passStyle, check) => {
const reject = !!check && (details => check(false, details));
return (
(isObject(tagRecord) ||
@@ -140,21 +156,35 @@ export const checkTagRecord = (tagRecord, passStyle, check) => {
(isFrozen(tagRecord) ||
(reject && reject(X`A tagRecord must be frozen: ${tagRecord}`))) &&
(!isArray(tagRecord) ||
- (reject && reject(X`An array cannot be a tagRecords: ${tagRecord}`))) &&
+ (reject && reject(X`An array cannot be a tagRecord: ${tagRecord}`))) &&
checkNormalProperty(tagRecord, PASS_STYLE, false, check) &&
- (tagRecord[PASS_STYLE] === passStyle ||
- (reject &&
- reject(
- X`Expected ${q(passStyle)}, not ${q(
- tagRecord[PASS_STYLE],
- )}: ${tagRecord}`,
- ))) &&
+ checkPassStyle(tagRecord, passStyle, check) &&
checkNormalProperty(tagRecord, Symbol.toStringTag, false, check) &&
(typeof getTag(tagRecord) === 'string' ||
(reject &&
reject(
X`A [Symbol.toStringTag]-named property must be a string: ${tagRecord}`,
- )))
+ ))) &&
+ checkProto(tagRecord, getPrototypeOf(tagRecord), check)
);
};
-harden(checkTagRecord);
+ return harden(checkTagRecord);
+};
+
+export const checkTagRecord = makeCheckTagRecord(
+ (val, proto, check) =>
+ proto === objectPrototype ||
+ (!!check &&
+ check(false, X`A tagRecord must inherit from Object.prototype: ${val}`)),
+);
+
+export const checkFunctionTagRecord = makeCheckTagRecord(
+ (val, proto, check) =>
+ proto === functionPrototype ||
+ (proto !== null && getPrototypeOf(proto) === functionPrototype) ||
+ (!!check &&
+ check(
+ false,
+ X`For functions, a tagRecord must inherit from Function.prototype: ${val}`,
+ )),
+);

MODIFY
packages/marshal/src/helpers/remotable.js
packages/marshal/src/helpers/remotable.js
@@ -8,6 +8,7 @@ import {
hasOwnPropertyOf,
PASS_STYLE,
checkTagRecord,
+ checkFunctionTagRecord,
isObject,
getTag,
} from './passStyle-helpers.js';
@@ -20,7 +21,6 @@ import {
const { details: X, quote: q } = assert;
const { ownKeys } = Reflect;
-const { prototype: functionPrototype } = Function;
const { isArray } = Array;
const {
getPrototypeOf,
@@ -67,15 +67,18 @@ export const assertIface = iface => checkIface(iface, assertChecker);
harden(assertIface);
/**
- * @param {any} original
+ * @param {object | Function} original
* @param {Checker} [check]
* @returns {boolean}
*/
const checkRemotableProtoOf = (original, check) => {
const reject = !!check && (details => check(false, details));
+ isObject(original) ||
+ assert.fail(X`Remotables must be objects or functions: ${original}`);
+
// A valid remotable object must inherit from a "tag record" -- a
// plain-object prototype consisting of only
- // a suitable `PASS_STYLE` property and a suitable `Symbol.toStringTag`
+ // a `PASS_STYLE` property with value "remotable" and a suitable `Symbol.toStringTag`
// property. The remotable could inherit directly from such a tag record, or
// it could inherit from another valid remotable, that therefore itself
// inherits directly or indirectly from such a tag record.
@@ -88,61 +91,26 @@ const checkRemotableProtoOf = (original, check) => {
// }}
//
const proto = getPrototypeOf(original);
- const protoProto = proto === null ? null : getPrototypeOf(proto);
- if (
- typeof original === 'object' &&
- proto !== objectPrototype &&
- protoProto !== objectPrototype &&
- protoProto !== null
- ) {
- return (
- // eslint-disable-next-line no-use-before-define
- RemotableHelper.canBeValid(proto, check) && checkRemotable(proto, check)
- );
- }
-
- // Since we're working with TypeScript's unsound type system, mostly
- // to catch accidents and to provide IDE support, we type arguments
- // like `val` according to what they are supposed to be. The following
- // tests for a particular violation. However, TypeScript complains
- // because *if the declared type were accurate*, then the condition
- // would always return true.
- // @ts-ignore TypeScript assumes what we're trying to check
- if (proto === objectPrototype) {
+ if (proto === objectPrototype || proto === null) {
return (
reject &&
reject(X`Remotables must be explicitly declared: ${q(original)}`)
);
}
- if (!checkTagRecord(proto, 'remotable', check)) {
- return false;
- }
if (typeof original === 'object') {
- const valid = protoProto === objectPrototype || protoProto === null;
- if (!valid) {
- return (
- reject &&
- reject(
- X`The Remotable Proto marker cannot inherit from anything unusual`,
- )
- );
+ const protoProto = getPrototypeOf(proto);
+ if (protoProto !== objectPrototype && protoProto !== null) {
+ // eslint-disable-next-line no-use-before-define
+ return checkRemotable(proto, check);
+ }
+ if (!checkTagRecord(proto, 'remotable', check)) {
+ return false;
}
} else if (typeof original === 'function') {
- const valid =
- protoProto === functionPrototype ||
- getPrototypeOf(protoProto) === functionPrototype;
- if (!valid) {
- return (
- reject &&
- reject(
- X`For far functions, the Remotable Proto marker must inherit from Function.prototype, in ${original}`,
- )
- );
+ if (!checkFunctionTagRecord(proto, 'remotable', check)) {
+ return false;
}
- } else {
- // XXX Should this be reject instead?
- assert.fail(X`unrecognized typeof ${original}`);
}
// Typecasts needed due to https://github.com/microsoft/TypeScript/issues/1863
@@ -182,16 +150,12 @@ const checkRemotable = (val, check) => {
return checkRemotableProtoOf(val, check);
};
-const isRemotable = val => checkRemotable(val, x => x);
-
/** @type {MarshalGetInterfaceOf} */
export const getInterfaceOf = val => {
- const typestr = typeof val;
if (
- (typestr !== 'object' && typestr !== 'function') ||
- val === null ||
+ !isObject(val) ||
val[PASS_STYLE] !== 'remotable' ||
- !isRemotable(val)
+ !checkRemotable(val)
) {
return undefined;
}
@@ -267,10 +231,7 @@ export const RemotableHelper = harden({
}
},
- assertValid: candidate => {
- RemotableHelper.canBeValid(candidate, assertChecker);
- checkRemotable(candidate, assertChecker);
- },
+ assertValid: candidate => checkRemotable(candidate, assertChecker),
every: (_passable, _fn) => true,
});

MODIFY
packages/marshal/src/helpers/tagged.js
packages/marshal/src/helpers/tagged.js
@@ -7,15 +7,12 @@ import {
checkTagRecord,
PASS_STYLE,
checkNormalProperty,
+ checkPassStyle,
} from './passStyle-helpers.js';
const { details: X } = assert;
const { ownKeys } = Reflect;
-const {
- getOwnPropertyDescriptors,
- getPrototypeOf,
- prototype: objectPrototype,
-} = Object;
+const { getOwnPropertyDescriptors } = Object;
/**
*
@@ -24,12 +21,10 @@ const {
export const TaggedHelper = harden({
styleName: 'tagged',
- canBeValid: (candidate, check) => checkTagRecord(candidate, 'tagged', check),
+ canBeValid: (candidate, check) => checkPassStyle(candidate, 'tagged', check),
assertValid: (candidate, passStyleOfRecur) => {
- TaggedHelper.canBeValid(candidate, assertChecker);
- getPrototypeOf(candidate) === objectPrototype ||
- assert.fail(X`Unexpected prototype for: ${candidate}`);
+ checkTagRecord(candidate, 'tagged', assertChecker);
// Typecasts needed due to https://github.com/microsoft/TypeScript/issues/1863
const passStyleKey = /** @type {unknown} */ (PASS_STYLE);

MODIFY
packages/marshal/src/make-far.js
packages/marshal/src/make-far.js
@@ -39,7 +39,6 @@ const makeRemotableProto = (remotable, iface) => {
oldProto = objectPrototype;
}
oldProto === objectPrototype ||
- oldProto === null ||
assert.fail(
X`For now, remotables cannot inherit from anything unusual, in ${remotable}`,
);

MODIFY
packages/marshal/test/test-marshal-capdata.js
packages/marshal/test/test-marshal-capdata.js
@@ -338,8 +338,9 @@ test('records', t => {
function shouldThrow(opts, message = /XXX/) {
t.throws(() => ser(build(...opts)), { message });
}
- const REC_NOACC = /must not be an accessor property:/;
- const REC_ONLYENUM = /must be an enumerable property:/;
+ const ERR_NOACCESSORS = /must not be an accessor property:/;
+ const ERR_ONLYENUMERABLE = /must be an enumerable property:/;
+ const ERR_REMOTABLE = /cannot serialize Remotables/;
// empty objects
@@ -370,17 +371,19 @@ test('records', t => {
slots: [],
});
- // anything with getters is rejected
- shouldThrow(['enumStringGetData'], REC_NOACC);
- shouldThrow(['enumStringGetData', 'enumStringData'], REC_NOACC);
- shouldThrow(['enumStringGetFunc'], REC_NOACC);
- shouldThrow(['enumStringGetFunc', 'enumStringData'], REC_NOACC);
- shouldThrow(['enumStringSet'], REC_NOACC);
- shouldThrow(['enumStringSet', 'enumStringData'], REC_NOACC);
-
- // anything with non-enumerable properties is rejected
- shouldThrow(['nonenumStringData'], REC_ONLYENUM);
- shouldThrow(['nonenumStringData', 'enumStringData'], REC_ONLYENUM);
+ // anything with an accessor is rejected
+ shouldThrow(['enumStringGetData'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringGetData', 'enumStringData'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringSet'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringSet', 'enumStringData'], ERR_NOACCESSORS);
+
+ // anything with a non-enumerable property is rejected
+ shouldThrow(['nonenumStringData'], ERR_ONLYENUMERABLE);
+ shouldThrow(['nonenumStringData', 'enumStringData'], ERR_ONLYENUMERABLE);
+
+ // anything with a function-returning getter is treated as remotable
+ shouldThrow(['enumStringGetFunc'], ERR_REMOTABLE);
+ shouldThrow(['enumStringData', 'enumStringGetFunc'], ERR_REMOTABLE);
});
test('capdata proto problems', t => {

MODIFY
packages/marshal/test/test-marshal-smallcaps.js
packages/marshal/test/test-marshal-smallcaps.js
@@ -189,8 +189,9 @@ test('smallcaps records', t => {
function shouldThrow(opts, message = /XXX/) {
t.throws(() => ser(build(...opts)), { message });
}
- const REC_NOACC = /must not be an accessor property:/;
- const REC_ONLYENUM = /must be an enumerable property:/;
+ const ERR_NOACCESSORS = /must not be an accessor property:/;
+ const ERR_ONLYENUMERABLE = /must be an enumerable property:/;
+ const ERR_REMOTABLE = /cannot serialize Remotables/;
// empty objects
@@ -221,17 +222,19 @@ test('smallcaps records', t => {
slots: [],
});
- // anything with getters is rejected
- shouldThrow(['enumStringGetData'], REC_NOACC);
- shouldThrow(['enumStringGetData', 'enumStringData'], REC_NOACC);
- shouldThrow(['enumStringGetFunc'], REC_NOACC);
- shouldThrow(['enumStringGetFunc', 'enumStringData'], REC_NOACC);
- shouldThrow(['enumStringSet'], REC_NOACC);
- shouldThrow(['enumStringSet', 'enumStringData'], REC_NOACC);
+ // anything with an accessor is rejected
+ shouldThrow(['enumStringGetData'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringGetData', 'enumStringData'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringSet'], ERR_NOACCESSORS);
+ shouldThrow(['enumStringSet', 'enumStringData'], ERR_NOACCESSORS);
// anything with non-enumerable properties is rejected
- shouldThrow(['nonenumStringData'], REC_ONLYENUM);
- shouldThrow(['nonenumStringData', 'enumStringData'], REC_ONLYENUM);
+ shouldThrow(['nonenumStringData'], ERR_ONLYENUMERABLE);
+ shouldThrow(['nonenumStringData', 'enumStringData'], ERR_ONLYENUMERABLE);
+
+ // anything with a function-returning getter is treated as remotable
+ shouldThrow(['enumStringGetFunc'], ERR_REMOTABLE);
+ shouldThrow(['enumStringGetFunc', 'enumStringData'], ERR_REMOTABLE);
const desertTopping = harden({
'#tag': 'floor wax',

MODIFY
packages/marshal/test/test-passStyleOf.js
packages/marshal/test/test-passStyleOf.js
@@ -23,6 +23,7 @@ test('passStyleOf basic success cases', t => {
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(harden({})), 'copyRecord', 'empty plain object');
t.is(passStyleOf(makeTagged('unknown', undefined)), 'tagged');
t.is(passStyleOf(harden(Error('ok'))), 'error');
@@ -105,9 +106,11 @@ test('passStyleOf testing tagged records', t => {
for (const proto of [null, {}]) {
const tagRecordBadProto = makeTagRecordVariant(undefined, proto);
- t.throws(() => passStyleOf(harden(tagRecordBadProto)), {
- message: /Unexpected prototype/,
- });
+ t.throws(
+ () => passStyleOf(harden(tagRecordBadProto)),
+ { message: /A tagRecord must inherit from Object.prototype/ },
+ `quasi-tagRecord with ${proto} prototype`,
+ );
}
const tagRecordExtra = makeTagRecordVariant();
@@ -195,7 +198,7 @@ test('passStyleOf testing remotables', t => {
const farObj6 = harden({
__proto__: farObjProto6,
});
- t.is(passStyleOf(farObj6), 'remotable');
+ t.is(passStyleOf(farObj6), 'remotable', 'tagRecord grandproto is accepted');
// Our current agoric-sdk plans for far classes are to create a class-like
// abstraction, but not to actually use the JavaScript class syntax.
@@ -239,35 +242,30 @@ test('passStyleOf testing remotables', t => {
message: 'For now, remotables cannot inherit from anything unusual, in {}',
});
- const tagRecordA1 = Object.create(null, {
- [PASS_STYLE]: { value: 'remotable' },
- [Symbol.toStringTag]: { value: 'Alleged: null grandproto is fine' },
- });
- const farObjProtoA1 = harden({
- __proto__: tagRecordA1,
- });
- const farObjA1 = harden({
- __proto__: farObjProtoA1,
- });
- t.is(passStyleOf(farObjA1), 'remotable');
+ const unusualTagRecordProtoMessage = /A tagRecord must inherit from Object.prototype/;
- const tagRecordA2 = Object.create(null, {
- [PASS_STYLE]: { value: 'remotable' },
- [Symbol.toStringTag]: { value: 'Alleged: null grandproto is fine' },
- });
- const farObjA2 = harden({
- __proto__: tagRecordA2,
- });
- t.is(passStyleOf(farObjA2), 'remotable');
+ const tagRecordA1 = makeTagishRecord(
+ 'Alleged: null-proto tagRecord proto',
+ null,
+ );
+ const farObjA1 = harden({ __proto__: tagRecordA1 });
+ t.throws(
+ () => passStyleOf(farObjA1),
+ { message: unusualTagRecordProtoMessage },
+ 'null-proto-tagRecord proto is rejected',
+ );
- const tagRecordA3 = makeTagishRecord(
- 'Alleged: null grandproto is fine',
+ const tagRecordA2 = makeTagishRecord(
+ 'Alleged: null-proto tagRecord grandproto',
null,
);
- const farObjA3 = harden({
- __proto__: tagRecordA3,
- });
- t.is(passStyleOf(farObjA3), 'remotable');
+ const farObjProtoA2 = harden({ __proto__: tagRecordA2 });
+ const farObjA2 = harden({ __proto__: farObjProtoA2 });
+ t.throws(
+ () => passStyleOf(farObjA2),
+ { message: unusualTagRecordProtoMessage },
+ 'null-proto-tagRecord grandproto is rejected',
+ );
t.throws(() => passStyleOf(Object.prototype), {
message: 'cannot serialize Remotables with accessors like "toString" in {}',
@@ -287,7 +285,6 @@ test('passStyleOf testing remotables', t => {
const farObjProtoWithExtra = makeTagishRecord(
'Alleged: manually constructed',
- null,
);
Object.defineProperty(farObjProtoWithExtra, 'extra', { value: () => {} });
const badFarObjExtraProtoProp = harden({ __proto__: farObjProtoWithExtra });

fix(marshal)!: Reject null as a substitute for Object.prototype

Fixes #1324