Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ses,pass-style): use non-trapping integrity trait ponyfill for safety #2681

Draft
wants to merge 1 commit into
base: markm-no-trapping-shim
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/captp/src/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const reverseSlot = slot => {
};

/**
* @typedef {object} CapTPImportExportTables
* @typedef {object} CapTPImportExportTables
* @property {(value: any) => CapTPSlot} makeSlotForValue
* @property {(slot: CapTPSlot, iface: string | undefined) => any} makeValueForSlot
* @property {(slot: CapTPSlot) => boolean} hasImport
Expand All @@ -58,12 +58,12 @@ const reverseSlot = slot => {
* @property {(slot: CapTPSlot, value: any) => void} markAsExported
* @property {(slot: CapTPSlot) => void} deleteExport
* @property {() => void} didDisconnect
*
* @typedef {object} MakeCapTPImportExportTablesOptions
* @property {boolean} gcImports
* @property {(slot: CapTPSlot) => void} releaseSlot
* @property {(slot: CapTPSlot) => RemoteKit} makeRemoteKit
*
* @param {MakeCapTPImportExportTablesOptions} options
* @returns {CapTPImportExportTables}
*/
Expand Down
1 change: 1 addition & 0 deletions packages/marshal/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"@endo/errors": "workspace:^",
"@endo/eventual-send": "workspace:^",
"@endo/nat": "workspace:^",
"@endo/non-trapping-shim": "^0.1.0",
"@endo/pass-style": "workspace:^",
"@endo/promise-kit": "workspace:^"
},
Expand Down
9 changes: 5 additions & 4 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
// encodes to CapData, a JSON-representable data structure, and leaves it to
// the caller (`marshal.js`) to stringify it.

import { X, Fail, q } from '@endo/errors';
import { extraObjectMethods } from '@endo/non-trapping-shim';
import {
passStyleOf,
isErrorLike,
Expand All @@ -17,7 +19,6 @@ import {
nameForPassableSymbol,
passableSymbolForName,
} from '@endo/pass-style';
import { X, Fail, q } from '@endo/errors';

/** @import {Passable, RemotableObject} from '@endo/pass-style' */
/** @import {Encoding, EncodingUnion} from './types.js' */
Expand All @@ -30,8 +31,8 @@ const {
is,
entries,
fromEntries,
freeze,
} = Object;
const { suppressTrapping } = extraObjectMethods;

/**
* Special property name that indicates an encoding that needs special
Expand Down Expand Up @@ -176,10 +177,10 @@ export const makeEncodeToCapData = (encodeOptions = {}) => {
// We harden the entire capData encoding before we return it.
// `encodeToCapData` requires that its input be Passable, and
// therefore hardened.
// The `freeze` here is needed anyway, because the `rest` is
// The `suppressTrapping` here is needed anyway, because the `rest` is
// freshly constructed by the `...` above, and we're using it
// as imput in another call to `encodeToCapData`.
result.rest = encodeToCapDataRecur(freeze(rest));
result.rest = encodeToCapDataRecur(suppressTrapping(rest));
}
return result;
}
Expand Down
1 change: 1 addition & 0 deletions packages/pass-style/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@endo/env-options": "workspace:^",
"@endo/errors": "workspace:^",
"@endo/eventual-send": "workspace:^",
"@endo/non-trapping-shim": "^0.1.0",
"@endo/promise-kit": "workspace:^"
},
"devDependencies": {
Expand Down
7 changes: 6 additions & 1 deletion packages/pass-style/src/passStyle-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,20 @@
/** @import {PassStyle} from './types.js' */

import { X, q } from '@endo/errors';
import { extraObjectMethods } from '@endo/non-trapping-shim';

const { isArray } = Array;
const { prototype: functionPrototype } = Function;
const {
getOwnPropertyDescriptor,
getPrototypeOf,
hasOwnProperty: objectHasOwnProperty,
isFrozen,
prototype: objectPrototype,
isFrozen,
} = Object;
const { apply } = Reflect;
const { toStringTag: toStringTagSymbol } = Symbol;
const { isNonTrapping } = extraObjectMethods;

const typedArrayPrototype = getPrototypeOf(Uint8Array.prototype);
const typedArrayToStringTagDesc = getOwnPropertyDescriptor(
Expand Down Expand Up @@ -167,6 +169,9 @@ const makeCheckTagRecord = checkProto => {
CX(check)`A non-object cannot be a tagRecord: ${tagRecord}`)) &&
(isFrozen(tagRecord) ||
(!!check && CX(check)`A tagRecord must be frozen: ${tagRecord}`)) &&
(isNonTrapping(tagRecord) ||
(!!check &&
CX(check)`A tagRecord must be non-trapping: ${tagRecord}`)) &&
(!isArray(tagRecord) ||
(!!check && CX(check)`An array cannot be a tagRecord: ${tagRecord}`)) &&
checkPassStyle(
Expand Down
33 changes: 21 additions & 12 deletions packages/pass-style/src/passStyleOf.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

import { isPromise } from '@endo/promise-kit';
import { X, Fail, q, annotateError, makeError } from '@endo/errors';
import { isObject, isTypedArray, PASS_STYLE } from './passStyle-helpers.js';
import { extraObjectMethods } from '@endo/non-trapping-shim';

import { isObject, isTypedArray, PASS_STYLE } from './passStyle-helpers.js';
import { CopyArrayHelper } from './copyArray.js';
import { CopyRecordHelper } from './copyRecord.js';
import { TaggedHelper } from './tagged.js';
Expand All @@ -31,7 +32,8 @@ import { assertPassableString } from './string.js';
/** @typedef {Exclude<PassStyle, PrimitiveStyle | "promise">} HelperPassStyle */

const { ownKeys } = Reflect;
const { isFrozen, getOwnPropertyDescriptors, values } = Object;
const { getOwnPropertyDescriptors, values, isFrozen } = Object;
const { isNonTrapping } = extraObjectMethods;

/**
* @param {PassStyleHelper[]} passStyleHelpers
Expand Down Expand Up @@ -143,14 +145,17 @@ const makePassStyleOf = passStyleHelpers => {
if (inner === null) {
return 'null';
}
if (!isFrozen(inner)) {
assert.fail(
// TypedArrays get special treatment in harden()
// and a corresponding special error message here.
isTypedArray(inner)
? X`Cannot pass mutable typed arrays like ${inner}.`
: X`Cannot pass non-frozen objects like ${inner}. Use harden()`,
);
if (!isNonTrapping(inner)) {
if (!isFrozen(inner)) {
throw assert.fail(
// TypedArrays get special treatment in harden()
// and a corresponding special error message here.
isTypedArray(inner)
? X`Cannot pass mutable typed arrays like ${inner}.`
: X`Cannot pass non-frozen objects like ${inner}. Use harden()`,
);
}
throw Fail`Cannot pass non-trapping objects like ${inner}`;
}
if (isPromise(inner)) {
assertSafePromise(inner);
Expand All @@ -177,8 +182,12 @@ const makePassStyleOf = passStyleHelpers => {
return 'remotable';
}
case 'function': {
isFrozen(inner) ||
Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
if (!isNonTrapping(inner)) {
if (!isFrozen(inner)) {
throw Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
}
throw Fail`Cannot pass trapping objects like ${inner}. Use harden()`;
}
typeof inner.then !== 'function' ||
Fail`Cannot pass non-promise thenables`;
remotableHelper.assertValid(inner, passStyleOfRecur);
Expand Down
5 changes: 3 additions & 2 deletions packages/pass-style/src/remotable.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/// <reference types="ses"/>

import { Fail, q } from '@endo/errors';
import { extraObjectMethods } from '@endo/non-trapping-shim';
import {
assertChecker,
canBeMethod,
Expand All @@ -24,10 +25,10 @@ const { ownKeys } = Reflect;
const { isArray } = Array;
const {
getPrototypeOf,
isFrozen,
prototype: objectPrototype,
getOwnPropertyDescriptors,
} = Object;
const { isNonTrapping } = extraObjectMethods;

/**
* @param {InterfaceSpec} iface
Expand Down Expand Up @@ -154,7 +155,7 @@ const checkRemotable = (val, check) => {
if (confirmedRemotables.has(val)) {
return true;
}
if (!isFrozen(val)) {
if (!isNonTrapping(val)) {
return (
!!check && CX(check)`cannot serialize non-frozen objects like ${val}`
);
Expand Down
8 changes: 5 additions & 3 deletions packages/pass-style/src/safe-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import { isPromise } from '@endo/promise-kit';
import { q } from '@endo/errors';
import { extraObjectMethods } from '@endo/non-trapping-shim';
import { assertChecker, hasOwnPropertyOf, CX } from './passStyle-helpers.js';

/** @import {Checker} from './types.js' */

const { isFrozen, getPrototypeOf, getOwnPropertyDescriptor } = Object;
const { getPrototypeOf, getOwnPropertyDescriptor } = Object;
const { ownKeys } = Reflect;
const { toStringTag } = Symbol;
const { isNonTrapping } = extraObjectMethods;

/**
* @param {Promise} pr The value to examine
Expand Down Expand Up @@ -88,7 +90,7 @@ const checkPromiseOwnKeys = (pr, check) => {
if (
typeof val === 'object' &&
val !== null &&
isFrozen(val) &&
isNonTrapping(val) &&
getPrototypeOf(val) === Object.prototype
) {
const subKeys = ownKeys(val);
Expand Down Expand Up @@ -132,7 +134,7 @@ const checkPromiseOwnKeys = (pr, check) => {
*/
const checkSafePromise = (pr, check) => {
return (
(isFrozen(pr) || CX(check)`${pr} - Must be frozen`) &&
(isNonTrapping(pr) || CX(check)`${pr} - Must be frozen`) &&
(isPromise(pr) || CX(check)`${pr} - Must be a promise`) &&
(getPrototypeOf(pr) === Promise.prototype ||
CX(check)`${pr} - Must inherit from Promise.prototype: ${q(
Expand Down
8 changes: 5 additions & 3 deletions packages/pass-style/test/passStyleOf.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import test from '@endo/ses-ava/prepare-endo.js';

import { q } from '@endo/errors';
import { extraObjectMethods } from '@endo/non-trapping-shim';

import { passStyleOf } from '../src/passStyleOf.js';
import { Far, ToFarFunction } from '../src/make-far.js';
Expand All @@ -13,8 +14,9 @@ const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
global.harden
);

const { getPrototypeOf, defineProperty, freeze } = Object;
const { getPrototypeOf, defineProperty } = Object;
const { ownKeys } = Reflect;
const { suppressTrapping } = extraObjectMethods;

test('passStyleOf basic success cases', t => {
// Test in same order as `passStyleOf` for easier maintenance.
Expand Down Expand Up @@ -208,7 +210,7 @@ test('passStyleOf testing remotables', t => {
*
* @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385
*/
const farObj2 = freeze({ __proto__: tagRecord2 });
const farObj2 = suppressTrapping({ __proto__: tagRecord2 });
if (harden.isFake) {
t.is(passStyleOf(farObj2), 'remotable');
} else {
Expand Down Expand Up @@ -386,7 +388,7 @@ test('remotables - safety from the gibson042 attack', t => {
* 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 makeInput = () => suppressTrapping({ __proto__: mercurialProto });
const input1 = makeInput();
const input2 = makeInput();

Expand Down
3 changes: 2 additions & 1 deletion packages/ses/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@
"postpack": "git clean -f '*.d.ts*' '*.tsbuildinfo'"
},
"dependencies": {
"@endo/env-options": "workspace:^"
"@endo/env-options": "workspace:^",
"@endo/non-trapping-shim": "^0.1.0"
},
"devDependencies": {
"@endo/compartment-mapper": "workspace:^",
Expand Down
20 changes: 18 additions & 2 deletions packages/ses/src/make-hardener.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

// @ts-check

import { extraObjectMethods } from '@endo/non-trapping-shim';
import {
Set,
String,
Expand All @@ -30,7 +31,6 @@ import {
apply,
arrayForEach,
defineProperty,
freeze,
getOwnPropertyDescriptor,
getOwnPropertyDescriptors,
getPrototypeOf,
Expand All @@ -49,9 +49,12 @@ import {
FERAL_STACK_GETTER,
FERAL_STACK_SETTER,
isError,
isFrozen,
} from './commons.js';
import { assert } from './error/assert.js';

const { suppressTrapping } = extraObjectMethods;

/**
* @import {Harden} from '../types.js'
*/
Expand Down Expand Up @@ -128,6 +131,10 @@ const freezeTypedArray = array => {
* @returns {Harden}
*/
export const makeHardener = () => {
// TODO Get the native hardener to suppressTrapping at each step,
// rather than freeze. Until then, we cannot use it, which is *expensive*!
// TODO Comment out the following to skip the native hardener.
//
// Use a native hardener if possible.
if (typeof globalThis.harden === 'function') {
const safeHarden = globalThis.harden;
Expand Down Expand Up @@ -182,8 +189,17 @@ export const makeHardener = () => {
// Also throws if the object is an ArrayBuffer or any TypedArray.
if (isTypedArray(obj)) {
freezeTypedArray(obj);
if (isFrozen(obj)) {
// After `freezeTypedArray`, the typed array might actually be
// frozen if
// - it has no indexed properties
// - it is backed by an Immutable ArrayBuffer as proposed.
// In either case, this makes it a candidate to be made
// non-trapping.
suppressTrapping(obj);
}
} else {
freeze(obj);
suppressTrapping(obj);
}

// we rely upon certain commitments of Object.freeze and proxies here
Expand Down
5 changes: 4 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,7 @@ __metadata:
"@endo/init": "workspace:^"
"@endo/lockdown": "workspace:^"
"@endo/nat": "workspace:^"
"@endo/non-trapping-shim": "npm:^0.1.0"
"@endo/pass-style": "workspace:^"
"@endo/promise-kit": "workspace:^"
"@endo/ses-ava": "workspace:^"
Expand Down Expand Up @@ -702,7 +703,7 @@ __metadata:
languageName: unknown
linkType: soft

"@endo/non-trapping-shim@workspace:packages/non-trapping-shim":
"@endo/non-trapping-shim@npm:^0.1.0, @endo/non-trapping-shim@workspace:packages/non-trapping-shim":
version: 0.0.0-use.local
resolution: "@endo/non-trapping-shim@workspace:packages/non-trapping-shim"
dependencies:
Expand All @@ -721,6 +722,7 @@ __metadata:
"@endo/errors": "workspace:^"
"@endo/eventual-send": "workspace:^"
"@endo/init": "workspace:^"
"@endo/non-trapping-shim": "npm:^0.1.0"
"@endo/promise-kit": "workspace:^"
"@endo/ses-ava": "workspace:^"
"@fast-check/ava": "npm:^1.1.5"
Expand Down Expand Up @@ -8960,6 +8962,7 @@ __metadata:
"@endo/compartment-mapper": "workspace:^"
"@endo/env-options": "workspace:^"
"@endo/module-source": "workspace:^"
"@endo/non-trapping-shim": "npm:^0.1.0"
"@endo/test262-runner": "workspace:^"
ava: "npm:^6.1.3"
babel-eslint: "npm:^10.1.0"
Expand Down
Loading