Skip to content

Commit

Permalink
feat(ses,pass-style): use no-trapping integrity level for safety
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Jan 1, 2025
1 parent f7d527c commit 86f8a8c
Show file tree
Hide file tree
Showing 21 changed files with 154 additions and 54 deletions.
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
9 changes: 7 additions & 2 deletions packages/captp/src/trap.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Lifted mostly from `@endo/eventual-send/src/E.js`.

const { freeze } = Object;

/**
* Default implementation of Trap for near objects.
*
Expand Down Expand Up @@ -63,7 +65,10 @@ const TrapProxyHandler = (x, trapImpl) => {
export const makeTrap = trapImpl => {
const Trap = x => {
const handler = TrapProxyHandler(x, trapImpl);
return harden(new Proxy(() => {}, handler));
return new Proxy(
freeze(() => {}),
handler,
);
};

const makeTrapGetterProxy = x => {
Expand All @@ -77,7 +82,7 @@ export const makeTrap = trapImpl => {
return trapImpl.get(x, prop);
},
});
return new Proxy(Object.create(null), handler);
return new Proxy(freeze(Object.create(null)), handler);
};
Trap.get = makeTrapGetterProxy;

Expand Down
27 changes: 20 additions & 7 deletions packages/eventual-send/src/E.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -171,6 +171,13 @@ const makeEGetProxyHandler = (x, HandledPromise) =>
* @param {HandledPromiseConstructor} HandledPromise
*/
const makeE = HandledPromise => {
// Note the use of `freeze` rather than `harden` below. This is because
// `harden` now implies no-trapping, and we depend on proxies with these
// almost-empty targets to remain trapping for traps `get`, `apply`, and `set`
// which can still be interesting even when the target is frozen.
// `get` and `has`, if not naming an own property, are still general traps,
// which we rely on. `apply`, surprisingly perhaps, is free to ignore the
// target's call behavior and just do its own thing instead.
return harden(
assign(
/**
Expand All @@ -182,8 +189,12 @@ const makeE = HandledPromise => {
* @param {T} x target for method/function call
* @returns {ECallableOrMethods<RemoteFunctions<T>>} method/function call proxy
*/
// @ts-expect-error XXX typedef
x => harden(new Proxy(() => {}, makeEProxyHandler(x, HandledPromise))),
x =>
// @ts-expect-error XXX typedef
new Proxy(
freeze(() => {}),
makeEProxyHandler(x, HandledPromise),
),
{
/**
* E.get(x) returns a proxy on which you can get arbitrary properties.
Expand All @@ -198,8 +209,9 @@ const makeE = HandledPromise => {
*/
get: x =>
// @ts-expect-error XXX typedef
harden(
new Proxy(create(null), makeEGetProxyHandler(x, HandledPromise)),
new Proxy(
freeze(create(null)),
makeEGetProxyHandler(x, HandledPromise),
),

/**
Expand All @@ -224,8 +236,9 @@ const makeE = HandledPromise => {
*/
sendOnly: x =>
// @ts-expect-error XXX typedef
harden(
new Proxy(() => {}, makeESendOnlyProxyHandler(x, HandledPromise)),
new Proxy(
freeze(() => {}),
makeESendOnlyProxyHandler(x, HandledPromise),
),

/**
Expand Down
2 changes: 2 additions & 0 deletions packages/eventual-send/src/handled-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ export const makeHandledPromise = () => {
const { proxy: proxyOpts } = options;
let presence;
if (proxyOpts) {
// TODO for these cases, it will be unreasonably hard for all uses
// to avoid hardening the returned proxy.
const {
handler: proxyHandler,
target: proxyTarget,
Expand Down
13 changes: 10 additions & 3 deletions packages/exo/src/exo-makers.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ import { defendPrototype, defendPrototypeKit } from './exo-tools.js';
* @import {Amplify, ExoClassKitMethods, ExoClassMethods, FarClassOptions, Guarded, GuardedKit, ExoClassInterfaceGuardKit, IsInstance, KitContext, ExoClassInterfaceGuard, Methods, FacetName} from './types.js';
*/

const { create, seal, freeze, defineProperty, values } = Object;
const {
create,
seal,
defineProperty,
values,
// @ts-expect-error TS doesn't know this is on ObjectConstructor
suppressTrapping,
} = Object;

// Turn on to give each exo instance its own toStringTag value.
const LABEL_INSTANCES = environmentOptionsListHas('DEBUG', 'label-instances');
Expand Down Expand Up @@ -92,7 +99,7 @@ export const defineExoClass = (

// Be careful not to freeze the state record
/** @type {import('./types.js').ClassContext<ReturnType<I>,M>} */
const context = freeze({ state, self });
const context = suppressTrapping({ state, self });
contextMap.set(self, context);
if (finish) {
finish(context);
Expand Down Expand Up @@ -173,7 +180,7 @@ export const defineExoClassKit = (
});
context.facets = facets;
// Be careful not to freeze the state record
freeze(context);
suppressTrapping(context);
if (finish) {
finish(context);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/far/test/marshal-far-function.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" /,
});
});
Expand Down
7 changes: 4 additions & 3 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ const {
is,
entries,
fromEntries,
freeze,
// @ts-expect-error TS doesn't know this is on ObjectConstructor
suppressTrapping,
} = Object;

/**
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
18 changes: 16 additions & 2 deletions packages/marshal/src/marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand All @@ -23,7 +25,13 @@ const badArrayHandler = harden({
},
});

const badArray = harden(new Proxy(harden([]), badArrayHandler));
// Note the use of `freeze` rather than `harden` below. This is because
// `harden` now implies no-trapping, and we depend on proxies with these
// almost-empty targets to remain trapping for the `get` trap
// which can still be interesting even when the target is frozen.
// `get`, if not naming an own property, are still general traps,
// which we rely on.
const badArray = new Proxy(freeze([]), badArrayHandler);

const { serialize, unserialize } = makeMarshal(
doNotConvertValToSlot,
Expand All @@ -48,7 +56,13 @@ harden(stringify);
*/
const parse = str =>
unserialize(
harden({
// Note the use of `freeze` rather than `harden` below. This is because
// `harden` now implies no-trapping, and we depend on proxies with these
// almost-empty targets to remain trapping for the `get` trap
// which can still be interesting even when the target is frozen.
// `get`, if not naming an own property, are still general traps,
// which we rely on.
freeze({
body: str,
slots: badArray,
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/marshal/test/marshal-far-function.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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" /,
});
});
Expand Down
5 changes: 3 additions & 2 deletions packages/pass-style/src/passStyle-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ const {
getOwnPropertyDescriptor,
getPrototypeOf,
hasOwnProperty: objectHasOwnProperty,
isFrozen,
prototype: objectPrototype,
// @ts-expect-error TS does not yet have `isNoTrapping` on ObjectConstructor
isNoTrapping,
} = Object;
const { apply } = Reflect;
const { toStringTag: toStringTagSymbol } = Symbol;
Expand Down Expand Up @@ -165,7 +166,7 @@ const makeCheckTagRecord = checkProto => {
(isObject(tagRecord) ||
(!!check &&
CX(check)`A non-object cannot be a tagRecord: ${tagRecord}`)) &&
(isFrozen(tagRecord) ||
(isNoTrapping(tagRecord) ||
(!!check && CX(check)`A tagRecord must be frozen: ${tagRecord}`)) &&
(!isArray(tagRecord) ||
(!!check && CX(check)`An array cannot be a tagRecord: ${tagRecord}`)) &&
Expand Down
11 changes: 8 additions & 3 deletions packages/pass-style/src/passStyleOf.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ import { assertPassableString } from './string.js';
/** @typedef {Exclude<PassStyle, PrimitiveStyle | "promise">} HelperPassStyle */

const { ownKeys } = Reflect;
const { isFrozen, getOwnPropertyDescriptors, values } = Object;
const {
getOwnPropertyDescriptors,
values,
// @ts-expect-error TS does not yet have `isNoTrapping` on ObjectConstructor
isNoTrapping,
} = Object;

/**
* @param {PassStyleHelper[]} passStyleHelpers
Expand Down Expand Up @@ -143,7 +148,7 @@ const makePassStyleOf = passStyleHelpers => {
if (inner === null) {
return 'null';
}
if (!isFrozen(inner)) {
if (!isNoTrapping(inner)) {
assert.fail(
// TypedArrays get special treatment in harden()
// and a corresponding special error message here.
Expand Down Expand Up @@ -177,7 +182,7 @@ const makePassStyleOf = passStyleHelpers => {
return 'remotable';
}
case 'function': {
isFrozen(inner) ||
isNoTrapping(inner) ||
Fail`Cannot pass non-frozen objects like ${inner}. Use harden()`;
typeof inner.then !== 'function' ||
Fail`Cannot pass non-promise thenables`;
Expand Down
5 changes: 3 additions & 2 deletions packages/pass-style/src/remotable.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ const { ownKeys } = Reflect;
const { isArray } = Array;
const {
getPrototypeOf,
isFrozen,
prototype: objectPrototype,
getOwnPropertyDescriptors,
// @ts-expect-error TS does not yet have `isNoTrapping` on ObjectConstructor
isNoTrapping,
} = Object;

/**
Expand Down Expand Up @@ -154,7 +155,7 @@ const checkRemotable = (val, check) => {
if (confirmedRemotables.has(val)) {
return true;
}
if (!isFrozen(val)) {
if (!isNoTrapping(val)) {
return (
!!check && CX(check)`cannot serialize non-frozen objects like ${val}`
);
Expand Down
11 changes: 8 additions & 3 deletions packages/pass-style/src/safe-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import { assertChecker, hasOwnPropertyOf, CX } from './passStyle-helpers.js';

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

const { isFrozen, getPrototypeOf, getOwnPropertyDescriptor } = Object;
const {
getPrototypeOf,
getOwnPropertyDescriptor,
// @ts-expect-error TS does not yet have `isNoTrapping` on ObjectConstructor
isNoTrapping,
} = Object;
const { ownKeys } = Reflect;
const { toStringTag } = Symbol;

Expand Down Expand Up @@ -88,7 +93,7 @@ const checkPromiseOwnKeys = (pr, check) => {
if (
typeof val === 'object' &&
val !== null &&
isFrozen(val) &&
isNoTrapping(val) &&
getPrototypeOf(val) === Object.prototype
) {
const subKeys = ownKeys(val);
Expand Down Expand Up @@ -132,7 +137,7 @@ const checkPromiseOwnKeys = (pr, check) => {
*/
const checkSafePromise = (pr, check) => {
return (
(isFrozen(pr) || CX(check)`${pr} - Must be frozen`) &&
(isNoTrapping(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
10 changes: 5 additions & 5 deletions packages/pass-style/test/passStyleOf.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const harden = /** @type {import('ses').Harden & { isFake?: boolean }} */ (
global.harden
);

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

test('passStyleOf basic success cases', t => {
Expand Down Expand Up @@ -200,7 +200,7 @@ test('passStyleOf testing remotables', t => {

const tagRecord2 = makeTagishRecord('Alleged: tagRecord not hardened');
/** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
const farObj2 = Object.freeze({
const farObj2 = suppressTrapping({
__proto__: tagRecord2,
});
if (harden.isFake) {
Expand All @@ -212,11 +212,11 @@ test('passStyleOf testing remotables', t => {
});
}

const tagRecord3 = Object.freeze(
const tagRecord3 = suppressTrapping(
makeTagishRecord('Alleged: both manually frozen'),
);
/** @type {any} UNTIL https://github.com/microsoft/TypeScript/issues/38385 */
const farObj3 = Object.freeze({
const farObj3 = suppressTrapping({
__proto__: tagRecord3,
});
t.is(passStyleOf(farObj3), 'remotable');
Expand Down Expand Up @@ -387,7 +387,7 @@ test('remotables - safety from the gibson042 attack', t => {
},
);

const makeInput = () => Object.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/no-trapping-shim": "^0.1.0"
},
"devDependencies": {
"@endo/compartment-mapper": "workspace:^",
Expand Down
Loading

0 comments on commit 86f8a8c

Please sign in to comment.