diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index afdd321d79..f0135d4274 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,61 @@ jobs: - name: Run yarn test run: yarn test + test-async-hooks: + name: test-async-hooks + + runs-on: ${{ matrix.platform }} + strategy: + fail-fast: false + matrix: + node-version: + - '14.2' # last version before promise fast-path without destroy + - '14.17' # last version before unconditional promise fast-path + - '14.18' # first version after unconditional promise fast-path + - '16.1' # last version before some significant promise hooks changes + - '16.5' # last version before unconditional promise fast-path + - '16.6' # first version after unconditional promise fast-path + platform: + - ubuntu-latest + +# begin macro + + steps: + + - name: Checkout + uses: actions/checkout@v2 + + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} + + - name: Echo node version + run: node --version + + - name: Get yarn cache directory path + id: yarn-cache-dir-path + run: echo "::set-output name=dir::$(yarn cache dir)" + + - name: Cache npm modules + uses: actions/cache@v1 + id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`) + with: + path: ${{ steps.yarn-cache-dir-path.outputs.dir }} + key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} + +# end macro + + - name: Install dependencies + run: yarn --frozen-lockfile --ignore-engines + + - name: Run yarn build + run: yarn build + + - name: Run yarn test (@endo/init) + working-directory: packages/init + run: yarn test + cover: name: cover diff --git a/packages/init/jsconfig.json b/packages/init/jsconfig.json new file mode 100644 index 0000000000..b2a58dc298 --- /dev/null +++ b/packages/init/jsconfig.json @@ -0,0 +1,18 @@ +{ + "compilerOptions": { + "target": "esnext", + "module": "esnext", + "noEmit": true, + "downlevelIteration": true, + "strictNullChecks": true, + "moduleResolution": "node" + }, + "include": [ + "*.js", + "*.ts", + "src/**/*.js", + "src/**/*.ts", + "test/**/*.js", + "test/**/*.ts" + ] +} diff --git a/packages/init/node-async_hooks-patch.js b/packages/init/node-async_hooks-patch.js new file mode 100644 index 0000000000..cfa3e6d83b --- /dev/null +++ b/packages/init/node-async_hooks-patch.js @@ -0,0 +1,3 @@ +import { setup } from './node-async_hooks.js'; + +setup({ withDestroy: true }); diff --git a/packages/init/node-async_hooks.js b/packages/init/node-async_hooks.js new file mode 100644 index 0000000000..5949c8945f --- /dev/null +++ b/packages/init/node-async_hooks.js @@ -0,0 +1,235 @@ +import { createHook, AsyncResource } from 'async_hooks'; + +const asyncHooksSymbols = { + async_id_symbol: undefined, + trigger_async_id_symbol: undefined, + destroyed: undefined, +}; + +const promiseAsyncHookFallbackStates = new WeakMap(); + +const setAsyncSymbol = (description, symbol) => { + if (!(description in asyncHooksSymbols)) { + throw new Error('Unknown symbol'); + } else if (!asyncHooksSymbols[description]) { + if (symbol.description !== description) { + // Throw an error since the whitelist mechanism relies on the description + throw new Error( + `Mismatched symbol found for ${description}: ${String(symbol)}`, + ); + } + asyncHooksSymbols[description] = symbol; + return true; + } else if (asyncHooksSymbols[description] !== symbol) { + // process._rawDebug( + // `Found duplicate ${description}:`, + // symbol, + // asyncHooksSymbols[description], + // ); + return false; + } else { + return true; + } +}; + +// We can get the `async_id_symbol` and `trigger_async_id_symbol` through a +// simple instantiation of async_hook.AsyncResource, which causes little side +// effects. These are the 2 symbols that may be late bound, aka after the promise +// is returned to the program and would normally be frozen. +const findAsyncSymbolsFromAsyncResource = () => { + let found = 0; + Object.getOwnPropertySymbols(new AsyncResource('Bootstrap')).forEach(sym => { + const { description } = sym; + if (description in asyncHooksSymbols) { + if (setAsyncSymbol(description, sym)) { + found += 1; + } + } + }); + return found; +}; + +// To get the `destroyed` symbol installed on promises by async_hooks, +// the only option is to create and enable an AsyncHook. +// Different versions of Node handle this in various ways. +const getPromiseFromCreateHook = () => { + const bootstrapHookData = []; + const bootstrapHook = createHook({ + init(asyncId, type, triggerAsyncId, resource) { + if (type !== 'PROMISE') return; + // process._rawDebug('Bootstrap', asyncId, triggerAsyncId, resource); + bootstrapHookData.push({ asyncId, triggerAsyncId, resource }); + }, + destroy(_asyncId) { + // Needs to be present to trigger the addition of the destroyed symbol + }, + }); + + bootstrapHook.enable(); + // Use a never resolving promise to avoid triggering settlement hooks + const trigger = new Promise(() => {}); + bootstrapHook.disable(); + + // In some versions of Node, async_hooks don't give access to the resource + // itself, but to a "wrapper" which is basically hooks metadata for the promise + const promisesData = bootstrapHookData.filter( + ({ resource }) => Promise.resolve(resource) === resource, + ); + const { length } = promisesData; + if (length > 1) { + // process._rawDebug('Found multiple potential candidates'); + } + + const promiseData = promisesData.find(({ resource }) => resource === trigger); + + if (promiseData) { + // Normally all promise hooks are disabled in a subsequent microtask + // That means Node versions that modify promises at init will still + // trigger our proto hooks for promises created in this turn + // The following trick will disable the internal promise init hook + // However, only do this for destroy modifying versions, since some versions + // only modify promises if no destroy hook is requested, and do not correctly + // reset the internal init promise hook in those case. (e.g. v14.16.2) + const resetHook = createHook({}); + resetHook.enable(); + resetHook.disable(); + } else if (length) { + // process._rawDebug('No candidates matched'); + } + return promiseData; +}; + +const findAsyncSymbolsFromPromiseCreateHook = () => { + const bootstrapPromise = getPromiseFromCreateHook(); + + if (bootstrapPromise) { + const { asyncId, triggerAsyncId, resource } = bootstrapPromise; + const symbols = Object.getOwnPropertySymbols(resource); + // const { length } = symbols; + let found = 0; + // if (length !== 3) { + // process._rawDebug(`Found ${length} symbols on promise:`, ...symbols); + // } + symbols.forEach(symbol => { + const value = resource[symbol]; + let type; + if (value === asyncId) { + type = 'async_id_symbol'; + } else if (value === triggerAsyncId) { + type = 'trigger_async_id_symbol'; + } else if (typeof value === 'object' && 'destroyed' in value) { + type = 'destroyed'; + } else { + // process._rawDebug(`Unexpected symbol`, symbol); + return; + } + + if (setAsyncSymbol(type, symbol)) { + found += 1; + } + }); + return found; + } else { + // This node version is not mutating promises + return -2; + } +}; + +const getAsyncHookFallbackState = (promise, { create = false } = {}) => { + let state = promiseAsyncHookFallbackStates.get(promise); + if (!state && create) { + state = { + [asyncHooksSymbols.async_id_symbol]: undefined, + [asyncHooksSymbols.trigger_async_id_symbol]: undefined, + }; + if (asyncHooksSymbols.destroyed) { + state[asyncHooksSymbols.destroyed] = undefined; + } + promiseAsyncHookFallbackStates.set(promise, state); + } + return state; +}; + +const setAsyncIdFallback = (promise, symbol, value) => { + const state = getAsyncHookFallbackState(promise, { create: true }); + + if (state[symbol]) { + if (state[symbol] !== value) { + // This can happen if a frozen promise created before hooks were enabled + // is used multiple times as a parent promise + // It's safe to ignore subsequent values + } + } else { + state[symbol] = value; + } +}; + +const getAsyncHookSymbolPromiseProtoDesc = ( + symbol, + { disallowGet = false } = {}, +) => ({ + set(value) { + if (Object.isExtensible(this)) { + Object.defineProperty(this, symbol, { + value, + writable: false, + configurable: false, + enumerable: false, + }); + } else { + // process._rawDebug('fallback set of async id', symbol, value, new Error().stack); + setAsyncIdFallback(this, symbol, value); + } + }, + get() { + if (disallowGet) { + return undefined; + } + const state = getAsyncHookFallbackState(this, { create: false }); + return state && state[symbol]; + }, + enumerable: false, + configurable: true, +}); + +export const setup = ({ withDestroy = true } = {}) => { + if (withDestroy) { + findAsyncSymbolsFromPromiseCreateHook(); + } else { + findAsyncSymbolsFromAsyncResource(); + } + + if ( + !asyncHooksSymbols.async_id_symbol || + !asyncHooksSymbols.trigger_async_id_symbol + ) { + // process._rawDebug(`Async symbols not found, moving on`); + return; + } + + const PromiseProto = Promise.prototype; + Object.defineProperty( + PromiseProto, + asyncHooksSymbols.async_id_symbol, + getAsyncHookSymbolPromiseProtoDesc(asyncHooksSymbols.async_id_symbol), + ); + Object.defineProperty( + PromiseProto, + asyncHooksSymbols.trigger_async_id_symbol, + getAsyncHookSymbolPromiseProtoDesc( + asyncHooksSymbols.trigger_async_id_symbol, + ), + ); + + if (asyncHooksSymbols.destroyed) { + Object.defineProperty( + PromiseProto, + asyncHooksSymbols.destroyed, + getAsyncHookSymbolPromiseProtoDesc(asyncHooksSymbols.destroyed, { + disallowGet: true, + }), + ); + } else if (withDestroy) { + // process._rawDebug(`Couldn't find destroyed symbol to setup trap`); + } +}; diff --git a/packages/init/package.json b/packages/init/package.json index e66bc248e8..e3562a603e 100644 --- a/packages/init/package.json +++ b/packages/init/package.json @@ -7,7 +7,10 @@ "exports": { ".": "./index.js", "./debug.js": "./debug.js", - "./pre.js": "./pre.js", + "./pre.js": { + "node": "./pre-node.js", + "default": "./pre.js" + }, "./pre-remoting.js": "./pre-remoting.js", "./pre-bundle-source.js": "./pre-bundle-source.js", "./package.json": "./package.json" @@ -21,6 +24,8 @@ "lint": "eslint '**/*.js'" }, "devDependencies": { + "@endo/compartment-mapper": "0.7.1", + "@endo/ses-ava": "0.2.21", "ava": "^3.12.1" }, "dependencies": { diff --git a/packages/init/pre-bundle-source.js b/packages/init/pre-bundle-source.js index 400d329cd3..85f432bb0c 100644 --- a/packages/init/pre-bundle-source.js +++ b/packages/init/pre-bundle-source.js @@ -4,4 +4,6 @@ // or if further vetted shim initialization is needed: // import '@endo/init/pre.js'; -import './pre.js'; +// Use a package self-reference to go through the "exports" resolution +// eslint-disable-next-line import/no-extraneous-dependencies +import '@endo/init/pre.js'; diff --git a/packages/init/pre-node.js b/packages/init/pre-node.js new file mode 100644 index 0000000000..88af15e0f0 --- /dev/null +++ b/packages/init/pre-node.js @@ -0,0 +1,4 @@ +// Generic preamble for all shims. + +import './node-async_hooks-patch.js'; +import '@endo/lockdown'; diff --git a/packages/init/pre-remoting.js b/packages/init/pre-remoting.js index 5e1b21e230..52efb6426f 100644 --- a/packages/init/pre-remoting.js +++ b/packages/init/pre-remoting.js @@ -1,5 +1,7 @@ // pre-remoting.js - shims necessary to use @endo/far -import './pre.js'; +// Use a package self-reference to go through the "exports" resolution +// eslint-disable-next-line import/no-extraneous-dependencies +import '@endo/init/pre.js'; export * from '@endo/eventual-send/shim.js'; diff --git a/packages/init/pre.js b/packages/init/pre.js index 134a7a87a5..c02dfb1c9c 100644 --- a/packages/init/pre.js +++ b/packages/init/pre.js @@ -1,2 +1,3 @@ // Generic preamble for all shims. + import '@endo/lockdown'; diff --git a/packages/init/test/.eslintrc.json b/packages/init/test/.eslintrc.json new file mode 100644 index 0000000000..5a0565b8cb --- /dev/null +++ b/packages/init/test/.eslintrc.json @@ -0,0 +1,5 @@ +{ + "rules": { + "import/no-extraneous-dependencies": ["error", { "devDependencies": true }] + } +} diff --git a/packages/init/test/fixture/ses-boot.js b/packages/init/test/fixture/ses-boot.js new file mode 100644 index 0000000000..9e9c13616b --- /dev/null +++ b/packages/init/test/fixture/ses-boot.js @@ -0,0 +1,2 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import '@endo/init'; diff --git a/packages/init/test/test-async_hooks.js b/packages/init/test/test-async_hooks.js new file mode 100644 index 0000000000..2f16211ab0 --- /dev/null +++ b/packages/init/test/test-async_hooks.js @@ -0,0 +1,77 @@ +/* global globalThis, $262 */ + +// Use a package self-reference to go through the "exports" resolution +// eslint-disable-next-line import/no-extraneous-dependencies +import '@endo/init'; +import test from 'ava'; +import { createHook } from 'async_hooks'; +import { setTimeout } from 'timers'; + +const gcP = (async () => { + let gc = globalThis.gc || (typeof $262 !== 'undefined' ? $262.gc : null); + if (!gc) { + gc = () => { + Array.from({ length: 2 ** 24 }, () => Math.random()); + }; + } + return gc; +})(); + +test('async_hooks Promise patch', async t => { + const hasAsyncSymbols = + Object.getOwnPropertySymbols(Promise.prototype).length > 1; + let resolve; + const q = (() => { + const p1 = new Promise(r => (resolve = r)); + t.deepEqual( + Reflect.ownKeys(p1), + [], + `Promise instances don't start with any own keys`, + ); + harden(p1); + + // The `.then()` fulfillment triggers the "before" hook for `p2`, + // which enforces that `p2` is a tracked promise by installing async id symbols + const p2 = Promise.resolve().then(() => {}); + t.deepEqual( + Reflect.ownKeys(p2), + [], + `Promise instances don't start with any own keys`, + ); + harden(p2); + + const testHooks = createHook({ + init() {}, + before() {}, + // after() {}, + destroy() {}, + }); + testHooks.enable(); + + // Create a promise with symbols attached + const p3 = Promise.resolve(); + t.is(Reflect.ownKeys(p3).length > 0, hasAsyncSymbols); + + return Promise.resolve().then(() => { + resolve(8); + // ret is a tracked promise created from parent `p1` + // async_hooks will attempt to get the asyncId from `p1` + // which was created and frozen before the symbols were installed + const ret = p1.then(() => {}); + // Trigger attempting to get asyncId of `p1` again, which in current + // node versions will fail and generate a new one because of an own check + p1.then(() => {}); + + t.is(Reflect.ownKeys(ret).length > 0, hasAsyncSymbols); + + // testHooks.disable(); + + return ret; + }); + })(); + + return q + .then(() => new Promise(r => setTimeout(r, 0, gcP))) + .then(gc => gc()) + .then(() => new Promise(r => setTimeout(r))); +}); diff --git a/packages/init/test/test-bundle.js b/packages/init/test/test-bundle.js new file mode 100644 index 0000000000..54c464a267 --- /dev/null +++ b/packages/init/test/test-bundle.js @@ -0,0 +1,25 @@ +// @ts-check + +import fs from 'fs'; +import url, { URL } from 'url'; +import crypto from 'crypto'; + +import '../debug.js'; +import { wrapTest } from '@endo/ses-ava'; +import rawTest from 'ava'; + +import { makeArchive } from '@endo/compartment-mapper'; +import { makeReadPowers } from '@endo/compartment-mapper/node-powers.js'; + +const test = wrapTest(rawTest); + +test('Can be bundled', async t => { + const powers = makeReadPowers({ fs, url, crypto }); + + await t.notThrowsAsync(() => + makeArchive( + powers, + new URL('./fixture/ses-boot.js', import.meta.url).toString(), + ), + ); +}); diff --git a/packages/ses/index.d.ts b/packages/ses/index.d.ts index d06b0993c0..c92cb06b56 100644 --- a/packages/ses/index.d.ts +++ b/packages/ses/index.d.ts @@ -28,6 +28,7 @@ export interface LockdownOptions { overrideTaming?: 'moderate' | 'min' | 'severe'; overrideDebug?: Array; domainTaming?: 'safe' | 'unsafe'; + promiseTaming?: 'safe' | 'unsafe'; __allowUnsafeMonkeyPatching__?: 'safe' | 'unsafe'; } diff --git a/packages/ses/src/lockdown-shim.js b/packages/ses/src/lockdown-shim.js index a9bc7ca78d..b70887d421 100644 --- a/packages/ses/src/lockdown-shim.js +++ b/packages/ses/src/lockdown-shim.js @@ -45,6 +45,7 @@ import { makeSafeEvaluator } from './make-safe-evaluator.js'; import { initialGlobalPropertyNames } from './whitelist.js'; import { tameFunctionToString } from './tame-function-tostring.js'; import { tameDomains } from './tame-domains.js'; +import { tamePromise } from './tame-promise.js'; import { tameConsole } from './error/tame-console.js'; import tameErrorConstructor from './error/tame-error-constructor.js'; @@ -166,6 +167,7 @@ export const repairIntrinsics = (options = {}) => { overrideTaming = getenv('LOCKDOWN_OVERRIDE_TAMING', 'moderate'), stackFiltering = getenv('LOCKDOWN_STACK_FILTERING', 'concise'), domainTaming = getenv('LOCKDOWN_DOMAIN_TAMING', 'safe'), + promiseTaming = getenv('LOCKDOWN_PROMISE_TAMING', 'safe'), evalTaming = getenv('LOCKDOWN_EVAL_TAMING', 'safeEval'), overrideDebug = arrayFilter( stringSplit(getenv('LOCKDOWN_OVERRIDE_DEBUG', ''), ','), @@ -377,6 +379,11 @@ export const repairIntrinsics = (options = {}) => { // @ts-ignore enablePropertyOverrides does its own input validation enablePropertyOverrides(intrinsics, overrideTaming, overrideDebug); + // tamePromise happens after enablePropertyOverrides and thus must + // cope with `Promise.prototype.constructor` being either + // a data property or an accessor property. + tamePromise(promiseTaming); + if (__allowUnsafeMonkeyPatching__ !== 'unsafe') { // Finally register and optionally freeze all the intrinsics. This // must be the operation that modifies the intrinsics. diff --git a/packages/ses/src/tame-promise.js b/packages/ses/src/tame-promise.js new file mode 100644 index 0000000000..d5dd500752 --- /dev/null +++ b/packages/ses/src/tame-promise.js @@ -0,0 +1,70 @@ +// @ts-check + +import { + Promise, + TypeError, + defineProperties, + objectHasOwnProperty, + freeze, + promisePrototype, + getPrototypeOf, + setPrototypeOf, + construct, +} from './commons.js'; + +const originalThen = promisePrototype.then; + +export const tamePromise = (promiseTaming = 'safe') => { + if (promiseTaming !== 'safe' && promiseTaming !== 'unsafe') { + throw new TypeError(`unrecognized promiseTaming ${promiseTaming}`); + } + if (promiseTaming === 'unsafe') { + return; + } + + function FakePromiseConstructor(...args) { + return construct(Promise, args, Promise); + } + defineProperties(FakePromiseConstructor, { + prototype: { + value: promisePrototype, + }, + }); + setPrototypeOf(FakePromiseConstructor, Promise); + freeze(FakePromiseConstructor); + + // At this point, enable-property-overrides may or may not have + // make `Promise.prototype.constructor` into an accessor. If it is + // a data property, we need to turn it into an accessor with the following + // getter and an `undefined` setter, so that after freezing it will + // have the same override mistake problem that the data property would + // have. + // If it is already an accessor property, we need to replace the getter with + // the following getter, but leave the setter that was presumably + // installed by enable-property-overrides. + const constructorDescDelta = { + get() { + freeze(this); + if ( + getPrototypeOf(this) === promisePrototype && + !objectHasOwnProperty(this, 'then') && + this.then === originalThen + ) { + return Promise; + } + return FakePromiseConstructor; + }, + }; + defineProperties(constructorDescDelta.get, { + originalValue: { + value: Promise, + writable: false, + enumerable: false, + configurable: false, + }, + }); + + defineProperties(promisePrototype, { + constructor: constructorDescDelta, + }); +}; diff --git a/packages/ses/src/whitelist.js b/packages/ses/src/whitelist.js index 1e97daa71c..8e8c1de908 100644 --- a/packages/ses/src/whitelist.js +++ b/packages/ses/src/whitelist.js @@ -1288,10 +1288,18 @@ export const whitelist = { '%PromisePrototype%': { // Properties of the Promise Prototype Object catch: fn, + // Note that tame-promise may turn this `constructor` property + // into an accessor. + // But it would do this after `whitelistIntrinsics` so 'Promise' + // remains the correct setting here in the whitelist. constructor: 'Promise', finally: fn, then: fn, '@@toStringTag': 'string', + // Non-standard, used in node to prevent async_hooks from breaking + 'UniqueSymbol(async_id_symbol)': accessor, + 'UniqueSymbol(trigger_async_id_symbol)': accessor, + 'UniqueSymbol(destroyed)': accessor, }, '%InertAsyncFunction%': { diff --git a/packages/ses/test/test-tame-promise-min-override.js b/packages/ses/test/test-tame-promise-min-override.js new file mode 100644 index 0000000000..7eb004a65c --- /dev/null +++ b/packages/ses/test/test-tame-promise-min-override.js @@ -0,0 +1,54 @@ +// @ts-check + +import '../index.js'; +import test from 'ava'; + +const { setPrototypeOf, defineProperty } = Object; + +lockdown({ promiseTaming: 'safe', overrideTaming: 'min' }); + +const normalPromise = Promise.resolve('normal'); +const thenable = { then: onSuccess => onSuccess('thenable') }; +const badOwnPromise = Promise.resolve('badOwnPromise'); +defineProperty(badOwnPromise, 'then', { + value: onSuccess => onSuccess('badThenResult'), +}); + +// Even though `badSubPromise` has no own `then` property, +// it still inherits a bad one. +const badSubPromise = Promise.resolve('badSubPromise'); +setPrototypeOf(badSubPromise, badOwnPromise); + +// Even though `badSpacerPromise.then` is the original `then` now, +// and it is not an own property, and it is inherited from +// the real Promise.prototype, it is still not reliable. +// The intermediate object on the prototype chain means that +// `then` be something else later. +const spacerProto = { __proto__: Promise.prototype }; +const badSpacerPromise = Promise.resolve('badSpacerPromise'); +setPrototypeOf(badSpacerPromise, spacerProto); + +test('tamePromise safe (min override) Promise.resolve', t => { + t.is(Promise.resolve(normalPromise), normalPromise); + // @ts-ignore It isn't supposed to match, so inconsistent types are ok. + t.not(Promise.resolve(thenable), thenable); + t.not(Promise.resolve(badOwnPromise), badOwnPromise); + t.not(Promise.resolve(badSubPromise), badSubPromise); + t.not(Promise.resolve(badSpacerPromise), badSpacerPromise); +}); + +const victim = p => { + let i = 'good'; + Promise.resolve(p).then(() => { + i = 'bad'; + }); + return i; +}; + +test('tamePromise safe (min override) reentrancy', t => { + t.is(victim(normalPromise), 'good'); + t.is(victim(thenable), 'good'); + t.is(victim(badOwnPromise), 'good'); + t.is(victim(badSubPromise), 'good'); + t.is(victim(badSpacerPromise), 'good'); +}); diff --git a/packages/ses/test/test-tame-promise-unsafe.js b/packages/ses/test/test-tame-promise-unsafe.js new file mode 100644 index 0000000000..7e46316d34 --- /dev/null +++ b/packages/ses/test/test-tame-promise-unsafe.js @@ -0,0 +1,57 @@ +// @ts-check + +import '../index.js'; +import test from 'ava'; + +const { setPrototypeOf, defineProperty } = Object; + +lockdown({ promiseTaming: 'unsafe' }); + +const normalPromise = Promise.resolve('normal'); +const thenable = { then: onSuccess => onSuccess('thenable') }; +const badOwnPromise = Promise.resolve('badOwnPromise'); +defineProperty(badOwnPromise, 'then', { + value: onSuccess => onSuccess('badThenResult'), +}); + +// Even though `badSubPromise` has no own `then` property, +// it still inherits a bad one. +const badSubPromise = Promise.resolve('badSubPromise'); +setPrototypeOf(badSubPromise, badOwnPromise); + +// Even though `badSpacerPromise.then` is the original `then` now, +// and it is not an own property, and it is inherited from +// the real Promise.prototype, it is still not reliable. +// The intermediate object on the prototype chain means that +// `then` be something else later. +const spacerProto = { __proto__: Promise.prototype }; +const badSpacerPromise = Promise.resolve('badSpacerPromise'); +setPrototypeOf(badSpacerPromise, spacerProto); + +test('tamePromise unsafe Promise.resolve', t => { + t.is(Promise.resolve(normalPromise), normalPromise); + // @ts-ignore It isn't supposed to match, so inconsistent types are ok. + t.not(Promise.resolve(thenable), thenable); + t.is(Promise.resolve(badOwnPromise), badOwnPromise); + t.is(Promise.resolve(badSubPromise), badSubPromise); + t.is(Promise.resolve(badSpacerPromise), badSpacerPromise); +}); + +const victim = p => { + let i = 'good'; + Promise.resolve(p).then(() => { + i = 'bad'; + }); + return i; +}; + +test('tamePromise unsafe reentrancy', t => { + t.is(victim(normalPromise), 'good'); + t.is(victim(thenable), 'good'); + t.is(victim(badOwnPromise), 'bad'); + t.is(victim(badSubPromise), 'bad'); + defineProperty(spacerProto, 'then', { + value: onSuccess => onSuccess('space attack'), + }); + t.is(victim(badSpacerPromise), 'bad'); +}); diff --git a/packages/ses/test/test-tame-promise.js b/packages/ses/test/test-tame-promise.js new file mode 100644 index 0000000000..d9527c2e8a --- /dev/null +++ b/packages/ses/test/test-tame-promise.js @@ -0,0 +1,54 @@ +// @ts-check + +import '../index.js'; +import test from 'ava'; + +const { setPrototypeOf, defineProperty } = Object; + +lockdown({ promiseTaming: 'safe' }); + +const normalPromise = Promise.resolve('normal'); +const thenable = { then: onSuccess => onSuccess('thenable') }; +const badOwnPromise = Promise.resolve('badOwnPromise'); +defineProperty(badOwnPromise, 'then', { + value: onSuccess => onSuccess('badThenResult'), +}); + +// Even though `badSubPromise` has no own `then` property, +// it still inherits a bad one. +const badSubPromise = Promise.resolve('badSubPromise'); +setPrototypeOf(badSubPromise, badOwnPromise); + +// Even though `badSpacerPromise.then` is the original `then` now, +// and it is not an own property, and it is inherited from +// the real Promise.prototype, it is still not reliable. +// The intermediate object on the prototype chain means that +// `then` be something else later. +const spacerProto = { __proto__: Promise.prototype }; +const badSpacerPromise = Promise.resolve('badSpacerPromise'); +setPrototypeOf(badSpacerPromise, spacerProto); + +test('tamePromise safe Promise.resolve', t => { + t.is(Promise.resolve(normalPromise), normalPromise); + // @ts-ignore It isn't supposed to match, so inconsistent types are ok. + t.not(Promise.resolve(thenable), thenable); + t.not(Promise.resolve(badOwnPromise), badOwnPromise); + t.not(Promise.resolve(badSubPromise), badSubPromise); + t.not(Promise.resolve(badSpacerPromise), badSpacerPromise); +}); + +const victim = p => { + let i = 'good'; + Promise.resolve(p).then(() => { + i = 'bad'; + }); + return i; +}; + +test('tamePromise safe reentrancy', t => { + t.is(victim(normalPromise), 'good'); + t.is(victim(thenable), 'good'); + t.is(victim(badOwnPromise), 'good'); + t.is(victim(badSubPromise), 'good'); + t.is(victim(badSpacerPromise), 'good'); +});