Skip to content

Commit

Permalink
security: force all handlers to run in a future turn
Browse files Browse the repository at this point in the history
fixes #19
  • Loading branch information
michaelfig committed Jul 13, 2019
1 parent b2e5d08 commit a401444
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 10 deletions.
32 changes: 22 additions & 10 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,25 @@ export default function maybeExtendPromise(Promise) {
// handled Promises to their corresponding fulfilledHandler.
let forwardingHandler;
function handler(p, operation, ...args) {
const h = promiseToHandler.get(p) || forwardingHandler;
if (typeof h[operation] !== 'function') {
const handlerName = h === forwardingHandler ? 'forwardingHandler' : 'unfulfilledHandler';
throw TypeError(`${handlerName}.${operation} is not a function`);
const unfulfilledHandler = promiseToHandler.get(p);
if (unfulfilledHandler) {
if (typeof unfulfilledHandler[operation] !== 'function') {
throw TypeError(`unfulfilledHandler.${operation} is not a function`);
}
// We pass the Promise directly, but we need to ensure we
// run in a future turn, to prevent synchronous attacks.
return Promise.resolve().then(() =>
unfulfilledHandler[operation](p, ...args),
);
}

// We use the forwardingHandler, but pass in the naked object in a future turn.
if (typeof forwardingHandler[operation] !== 'function') {
throw TypeError(`forwardingHandler.${operation} is not a function`);
}
return h[operation](p, ...args);
return Promise.resolve(p).then(o =>
forwardingHandler[operation](o, ...args),
);
}

Object.defineProperties(
Expand Down Expand Up @@ -210,19 +223,18 @@ export default function maybeExtendPromise(Promise) {
);

function makeForwarder(operation, localImpl) {
return async (ep, ...args) => {
const o = await ep;
return (o, ...args) => {
// We are in another turn already, and have the naked object.
const fulfilledHandler = presenceToHandler.get(o);
if (fulfilledHandler) {
// The handler was resolved, so give it a naked object.
// The handler was resolved, so use it.
if (typeof fulfilledHandler[operation] !== 'function') {
throw TypeError(`fulfilledHandler.${operation} is not a function`);
}
return fulfilledHandler[operation](o, ...args);
}

// Not a handled Promise, so use the local implementation on the
// naked object.
// Not handled, so use the local implementation.
return localImpl(o, ...args);
};
}
Expand Down
55 changes: 55 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,61 @@ if (typeof window !== 'undefined') {
});
}

test('handlers are always async', async t => {
try {
const EPromise = maybeExtendPromise(Promise);

const queue = [];
const handler = {
POST(_o, fn, args) {
queue.push([fn, args]);
return 'foo';
},
};
let resolver;
const ep = EPromise.makeHandled(resolve => {
resolver = resolve;
}, handler);

// Make sure asynchronous posts go through.
const firstPost = ep.post('myfn', ['abc', 123]).then(v => {
t.equal(v, 'foo', 'post return value is foo');
t.deepEqual(queue, [['myfn', ['abc', 123]]], 'single post in queue');
});

t.deepEqual(queue, [], 'unfulfilled post is asynchronous');
await firstPost;
t.deepEqual(
queue,
[['myfn', ['abc', 123]]],
'single post in queue after await',
);

const target = {};
resolver(target, handler);
const secondPost = ep.post('myotherfn', ['def', 456]).then(v => {
t.equal(v, 'foo', 'second post return value is foo');
t.deepEqual(
queue,
[['myfn', ['abc', 123]], ['myotherfn', ['def', 456]]],
'second post is queued',
);
});

t.deepEqual(queue, [['myfn', ['abc', 123]]], 'second post is asynchronous');
await secondPost;
t.deepEqual(
queue,
[['myfn', ['abc', 123]], ['myotherfn', ['def', 456]]],
'second post is queued after await',
);
} catch (e) {
t.assert(false, e);
} finally {
t.end();
}
});

test('maybeExtendPromise will not overwrite', async t => {
try {
const { makeHandled: secondMakeHandled } = maybeExtendPromise(Promise);
Expand Down

0 comments on commit a401444

Please sign in to comment.