doc: add note that vm module is not a security mechanism

[krydos]

the text added in this commit should warn users about
wrong idea that vm module can be secure to run unsafe scripts
in sandboxes

Checklist

• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc, vm

Hi,
As it mentioned in issue #11550 the VM module can be mistakenly used as a security mechanism and it should be great to have a note that VM module it is not about security.

[addaleax]

I think it would be really helpful to explain why this is hard to get right. (Afaict, it is not strictly impossible to use a VM context for actual sandboxing, just something that can go wrong easily. But then again I haven’t tried to actually do that for real.)

[jasnell]

Please line wrap at 80 chars.
Also, please avoid the use of informal pronouns like you.

[Trott]

How about something more like this?:

The `vm` module is not a security mechanism. Do not use it to run untrusted scripts.

[krydos]

@jasnell thank you for the review, required changes were made.

@Trott completely agree. In you case sentence looks more like tech doc.

@addaleax I think it is good idea to add info about why it is not security mechanism. Since it can take more space I think it should be somewhere at the bottom of the page. Maybe with link from top of the page. Also I'm not really sure how to explain why it is not security mechanism, since sandboxes (contexts) really makes impression of safety and secure. Please share you thoughts.

[ChALkeR]

Note that the issue is a bit deeper here, e.g. https://www.npmjs.com/package/vm2 is built on top of the vm module and positions itself as a secure sandbox, which it is not.

Perhaps it's worth to mention that anything that is built on vm and runs untrusted code in the same process is not secure, but I am not exactly sure how that should be worded.

[ChALkeR]

@addaleax

Afaict, it is not strictly impossible to use a VM context for actual sandboxing, just something that can go wrong easily

I'm pretty sure that it's impossible to use vm module alone (without kvm or at least a separate process) for sandboxing without parsing and verifying the code in question.

Btw, see #3020 — that perhaps needs a fix on the documentation side.

[seishun]

s/scripts/code/

[krydos]

@seishun sounds much better. Thank you. I pushed new changes.

[vkurchatkin]

I'm pretty sure that it's impossible to use vm module alone (without kvm or at least a separate process) for sandboxing without parsing and verifying the code in question

Well, browsers do just that, so it should be possible

[ChALkeR]

Well, browsers do just that, so it should be possible

No, it's not — the usecases are very different here. In the browser, you trigger a website open yourself, and if that crashes a tab or several ones (which is quite possible, btw.) — it's not a big deal actually. On a server — anyone could send you a malicious payload (without any additional action required) if you accept untrusted code and run it in vm. That can at least crash the process (e.g. by consuming all resources) and if that process is not a separate one — cause a DoS for the webserver.

Also (with lower significance, but this one of reasons why I would recommend not just a separate process, but a decent isolation): in the browser, the API surface that could be used by all the sites is pretty much limited, in Node.js — the main process has unrestricted access to the network/filesystem/etc. from the js code, so vm sandboxing becomes the only isolation layer between the untrusted code and a full system compromise. You shouldn't trust a single isolation layer when you can have two.

Also note that there are additional ways to shoot yourself in the foot here, like passing Buffer inside the vm context (that will allow untrusted code to read your data). I have seen people done that (in fact take a look at the vm2 npm module).

[vkurchatkin]

No, it's not — the usecases are very different here.

You won't be happy if an iframe chrashes the whole tab, so it's very similar

You shouldn't trust a single isolation layer when you can have two.

In that case you probably shouldn't trust two if you can have three. On the other hand, why use two if one does the job?

[ChALkeR]

You won't be happy if an iframe chrashes the whole tab, so it's very similar

That's quite possible today (try const x = []; for (;;) x.push(new ArrayBuffer(1e3)) for example), and the impact of a crashed tab that required a certain action from the user it affects (e.g. open a misbehaving page) is very different from crashing a webserver on user input without any action required from the server.

[Fishrock123]

Maybe bold this last line?

[krydos]

@Fishrock123 Sounds great. I will update pull request pretty soon. Thank you!

[bnoordhuis]

Well, browsers do just that, so it should be possible

Chrome goes to considerable lengths to run iframes in separate sandboxed processes. I'd say that underlines @ChALkeR's point of defense in depth.

It's possible to isolate contexts by giving them their own security token (something node doesn't currently do; in fact, it does the opposite) but that only protects you against prototype traversal, it's no good against a CPU or memory consumption denial-of-service.

[bnoordhuis]

LGTM but s/machanism/mechanism/ in the commit log.

[krydos]

I changed the typo in mechanism word of my first commit message and force pushed everything again. I hope it is ok, please correct me if I did something wrong.

[jasnell]

Landed in 2e74b0d.

Thank you @krydos! I squashed the commits on landing.