laverdet / isolated-vm

Secure & isolated JS environments for nodejs
ISC License
2.19k stars 154 forks source link

Pass objects from host to Isolated VM. (with Promises and HTTP requests) #396

Closed Valanap closed 1 year ago

Valanap commented 1 year ago

Hello, I'm working on solution that will let users run some API queries for data retrieval before it reaches the destination.

This is the minimal version what we designed 4 years ago, while vm2 package had critical security incident we are trying to replicate the same functionality in isolated-vm, previously in vm2 we could could pass entire object and freeze it, then run commands from the host without any issues, while this tool is meant to be internal only I assume it was okay at this point to use it.

We get the idea about isolation that it should be designed to run untrusted code and don't play with a host, however in our use case it is a must (as mentoined it is internal tool anyways) to have some predefined functions that user can only use, and some security layer that we add on top of the sandbox, e.g. they can just call function instead of parsing interceptors, loggers etc. as this is taken care of by the host.

So this is the minimal script that doesn't work, I already spend like a day trying to figure out the way of processing promises inside the sandbox, while primitives and simple functions e.g. emitter are not the issue, the promises and running code inherited from host is not the option as of now.

We wanted to keep the flow simple, share state between sandboxes using emitters (so we can access the previous values in next executions), and perform some API requests, getting their results to be passed to the emitter, while the emitter part works fine, making the HTTP call is the issue.

Sample of diagram what we expect to do, as example we took axios package wrapped into promise.

image

Kind request looking for your help getting this one resolved, we tried many options e.g. passing object, reference to function etc. but noone of these seem to be working as expected, also this code must be executed and returned to the user, so emitting operation is not the best fit there.

import { EventEmitter } from "events";
import ivm from "isolated-vm";
import * as axios from 'axios';

class HostObject {
    async httpCall(url, data) {
        const result = await axios.default({
            method: 'POST',
            url,
            data
        });
        return result.data;
    }
}

(async () => {

    const results = [];

    const emitter = new EventEmitter();
    emitter.on('event', (data) => {
        results.push(data);
    });

    const emit = (data) => {
        emitter.emit('event', data);
    }

    const isolate = new ivm.Isolate({});
    const context = isolate.createContextSync();
    context.global.setSync('emit', function(...args) {
        emit(...args);
    });

    const hostObjectRef = new HostObject();

    context.global.setSync(
        '__hostObject_httpCall',
        new ivm.Reference(async function (...args) {
            const result = await hostObjectRef.httpCall(args[0], args[1]);
            return new ivm.ExternalCopy(result);
        })
    );

    context.global.setSync(
        'hostObject',
        hostObjectRef,
        {
            reference: true
        }
    );

    const script = await isolate.compileScript(`
        (async()=>{ 

            const url = 'https://webhook.site/fc61ddef-.....................';
            const body = {};

            // TypeError: hostObject.httpCall is not a function
            await hostObject.httpCall(url, body);

            // TypeError: A non-transferable value was passed
            __hostObject_httpCall.applySyncPromise(undefined, [url, body], {});

            emit('x');

        })();     
    `);

    await script.run(context, {promise: true});

    console.log(results);
})();
laverdet commented 1 year ago

In the first case await hostObject.httpCall(url, body); this result is not surprising. You can't pass a "bag of functions" object to an isolate. One might also ask how to invoke this function from internet explorer while running nodejs. You can't, it's non-sense. You could do this the Reference constructor and unsafeInherit but I wouldn't recommend it.

Your second case is a lot closer __hostObject_httpCall.applySyncPromise(undefined, [url, body], {});. applySyncPromise is meant for functions which should appear to be synchronous but are implemented as an async function. For example this would allow you to implement fs.readFileSync in your isolate, but use fs.readFile on the nodejs side. The non-transferrable error is because body is an object which is not transferable, since it is not a primitive. Try __hostObject_httpCall.apply(undefined, [url, body], { arguments: { copy: true }, result: { promise: true }}); instead

Valanap commented 1 year ago

Answering to the first one, the code will be executed on server side only, it is not designed to run any code in vm, would you provide any example with Reference and unsafeInherit? As it seem to be used as server side only.

For the second one it is slighlty more complex to develop to the users, so ideally there is no clear way to simply await any functions directly in isolated even in async self-sexecuted method that exists outside of the isolation?

laverdet commented 1 year ago

There's nothing stopping you from doing: const hostObject = { httpCall: (url, body) => __hostObject_httpCall.apply(...theRest) } or whatever.

There's documentation for unsafeInherit in the readme