Skip to content

Commit

Permalink
fix: have liveSlots reject Promise arguments in D() invocations (#1803)
Browse files Browse the repository at this point in the history
This is in addition to the kernel-side translator killing the vat if one gets
through. Using a promise in `syscall.callNow()` is vat-fatal. Using one in
`D()` merely throws an Error (thrown by liveslots before the syscall is
made).

closes #1358
  • Loading branch information
warner committed Sep 30, 2020
1 parent c51c67b commit cdcf99d
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 3 deletions.
9 changes: 9 additions & 0 deletions packages/SwingSet/src/kernel/liveSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ function build(
return p;
}

function forbidPromises(serArgs) {
for (const slot of serArgs.slots) {
if (parseVatSlot(slot).type === 'promise') {
throw Error(`D() arguments cannot include a Promise`);
}
}
}

function DeviceHandler(slot) {
return {
get(target, prop) {
Expand All @@ -307,6 +315,7 @@ function build(
}
return (...args) => {
const serArgs = m.serialize(harden(args));
forbidPromises(serArgs);
const ret = syscall.callNow(slot, prop, serArgs);
insistCapData(ret);
const retval = m.unserialize(ret);
Expand Down
6 changes: 4 additions & 2 deletions packages/SwingSet/test/files-devices/bootstrap-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,19 @@ export function buildRootObject(vatPowers, vatParameters) {
const p = Promise.resolve();
log('sending Promise');
try {
// this will be rejected immediately, killing the vat shortly
// this will be rejected by liveslots before the device is involved
D(devices.d0).send({ p });
// shouldn't get here
log('oops: survived sending Promise');
} catch (e) {
// we aren't currently killed until the end of the crank
log('good: callNow failed');
}
} else {
throw new Error(`unknown argv mode '${argv[0]}'`);
}
},
ping() {
return true;
},
});
}
48 changes: 48 additions & 0 deletions packages/SwingSet/test/files-devices/bootstrap-4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { assert } from '@agoric/assert';
import { QCLASS } from '@agoric/marshal';
import { insistVatType } from '../../src/parseVatSlots';

// to exercise the error we get when syscall.callNow() is given a promise
// identifier, we must bypass liveslots, which would otherwise protect us
// against the vat-fatal mistake

function capdata(body, slots = []) {
return harden({ body, slots });
}

function capargs(args, slots = []) {
return capdata(JSON.stringify(args), slots);
}

export default function setup(syscall, state, _helpers, vatPowers) {
const { callNow } = syscall;
const { testLog } = vatPowers;
const dispatch = harden({
deliver(facetid, method, args, _result) {
if (method === 'bootstrap') {
// find the device slot
const [_vats, devices] = JSON.parse(args.body);
const qnode = devices.d0;
assert.equal(qnode[QCLASS], 'slot');
const slot = args.slots[qnode.index];
insistVatType('device', slot);

const vpid = 'p+1'; // pretend we're exporting a promise
const pnode = { [QCLASS]: 'slot', index: 0 };
const callNowArgs = capargs([pnode], [vpid]);

testLog('sending Promise');
try {
// this will throw an exception, but is also (eventually) vat-fatal
callNow(slot, 'send', callNowArgs);
testLog('oops: survived sending Promise');
} catch (e) {
testLog('good: callNow failed');
}
} else if (method === 'ping') {
testLog('oops: still alive');
}
},
});
return dispatch;
}
44 changes: 43 additions & 1 deletion packages/SwingSet/test/test-devices.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ test.before(async t => {
const bootstrap1 = await bundleSource(dfile('bootstrap-1'));
const bootstrap2 = await bundleSource(dfile('bootstrap-2'));
const bootstrap3 = await bundleSource(dfile('bootstrap-3'));
const bootstrap4 = await bundleSource(dfile('bootstrap-4'));
t.context.data = {
kernelBundles,
bootstrap0,
bootstrap1,
bootstrap2,
bootstrap3,
bootstrap4,
};
});

Expand Down Expand Up @@ -475,7 +477,7 @@ test.serial('command deliver', async t => {
t.deepEqual(rejection, { response: 'body' });
});

test('callNow refuses promises', async t => {
test.serial('liveslots throws when D() gets promise', async t => {
const config = {
bootstrap: 'bootstrap',
vats: {
Expand All @@ -494,6 +496,46 @@ test('callNow refuses promises', async t => {
await initializeSwingset(config, ['promise1'], storage, t.context.data);
const c = await makeSwingsetController(storage, { d0: {} });
await c.step();
// When liveslots catches an attempt to send a promise into D(), it throws
// a regular error, which the vat can catch.
t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']);

// If that isn't working as expected, and the promise makes it to
// syscall.callNow, the translator will notice and kill the vat. We send a
// ping() to it to make sure it's still alive. If the vat were dead,
// queueToVatExport would throw because the vat was deleted.
c.queueToVatExport('bootstrap', 'o+0', 'ping', capargs([]), 'panic');
await c.run();

// If the translator doesn't catch the promise and it makes it to the device,
// the kernel will panic, and the c.step() above will reject, so the
// 'await c.step()' will throw.
});

test.serial('syscall.callNow(promise) is vat-fatal', async t => {
const config = {
bootstrap: 'bootstrap',
vats: {
bootstrap: {
bundle: t.context.data.bootstrap4, // uses setup() to bypass liveslots
creationOptions: { enableSetup: true },
},
},
devices: {
d0: {
sourceSpec: require.resolve('./files-devices/device-0'),
},
},
};
const storage = initSwingStore().storage;
await initializeSwingset(config, [], storage, t.context.data);
const c = await makeSwingsetController(storage, { d0: {} });
await c.step();
// if the kernel paniced, that c.step() will reject, and the await will throw
t.deepEqual(c.dump().log, ['sending Promise', 'good: callNow failed']);
// now check that the vat was terminated: this should throw an exception
// because the entire bootstrap vat was deleted
t.throws(() => c.queueToVatExport('bootstrap', 'o+0', 'ping', capargs([])), {
message: `vat name bootstrap must exist, but doesn't`,
});
});

0 comments on commit cdcf99d

Please sign in to comment.