Skip to content
This repository was archived by the owner on Jan 1, 2025. It is now read-only.

is this secure or not? #325

Closed
cosbgn opened this issue Oct 25, 2020 · 4 comments
Closed

is this secure or not? #325

cosbgn opened this issue Oct 25, 2020 · 4 comments

Comments

@cosbgn
Copy link

cosbgn commented Oct 25, 2020

In the wiki I read:

If you're looking for a solution to run untrusted code, you should take a look at awesome isolated-vm library.

Yet in the readme I read:

m2 is a sandbox that can run untrusted code

So which one is it? Can I run untrusted code and be sure that my env.variables are safe? I need only to run plain JS and tiny-json-http or axios to make API requests, nothing else.

Thank you for the clarifications

@y21
Copy link
Contributor

y21 commented Oct 26, 2020

It is secure
vm2 uses node's vm module behind the scenes and (unlike native vm) you can't climb up to the host using constructor to obtain a reference to host Function. In native vm module, this.constructor.constructor gives you a reference to the host Function and would let attackers evaluate a JS string on the host process. If you decide to run untrusted code in the same process, you need to be careful when dealing with async and values coming from the sandbox since vm uses the same thread.

isolated-vm gives you access to V8 isolates, which run on another thread. That means infinite loops won't ever block your event loop. IVM also supports memory limits

You can use sandbox in the VM constructor to add axios to the global namespace. Then you will be able to use axios from within the VM.

const {VM} = require('vm2');
const axios = require('axios').default;

const vm = new VM({
    sandbox: {
        axios
    }
});

(async () => {
    const res = await vm.run(`
        axios.get('http://httpbin.org/get')
            .then(x => x.data)
            .then(x => x.url);
    `);

    console.log(res); // => http://httpbin.org/get
})();

@cosbgn
Copy link
Author

cosbgn commented Oct 26, 2020

Hi @y21 thank you very much for your answer and explanation.
Just one more question regarding

you need to be careful when dealing with async and values coming from the sandbox

I have an async function in my host, and I await the promise which is generated inside the sandbox. Is that fine? Something like this:

		const vm = new NodeVM({
			timeout: 4000,
			require: {
				external: ['axios'],
				builtin: false
			},
			sandbox:{
				logs:[],
				response:'',
				data:data
			},
			eval:false,
			wasm:false,
		});
		const resp = {value:null, error:null, logs:vm.sandbox.logs}
		try {
			vm.run(`
				const axios = require('axios');
				const log = (...l) => logs.push(...l);
				response = (async () => {${script}})();
			`, 'whatever_name_here_works_I_do_not_know_what_this_is.js');
		} 
		catch(e){
			resp.error = e.message
			return resp
		}
		try {
			resp.value = await vm.sandbox.response
		} catch(e){
			resp.error = e.message
		}
		return resp

Pretty much I'm saving a promise in sandbox.response with for example await axios.get(url) and then I'm awaiting it outside the sandbox (I do this because I can't figure out how to await in the sandbox).

Is this Ok? Thanks again!

@y21
Copy link
Contributor

y21 commented Oct 26, 2020

Just keep in mind that NodeVM is not immune to infinite loops, so timeout: 4000 won't work

IMPORTANT: Timeout is not effective for NodeVM so it is not immune to while (true) {} or similar evil.

I think the safest way to do this is to run vm2 in a separate process or to use worker threads. That way your main thread will never hang if someone tries to run while(true);
You can detect infinite loops by letting the process/worker thread periodically send "pings" to the parent process/thread using setInterval. If it hasn't received a ping in a while, it's probably stuck in an infinite loop. example
If you're already doing something like that where you have a "master" thread or process that controls vm2 instance(s) you're probably fine

@cosbgn
Copy link
Author

cosbgn commented Oct 26, 2020

Great thank you very much

@cosbgn cosbgn closed this as completed Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants