Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run extensions inside a sandbox #59756

Closed
NotWearingPants opened this issue Oct 1, 2018 · 1 comment
Closed

Run extensions inside a sandbox #59756

NotWearingPants opened this issue Oct 1, 2018 · 1 comment
Labels
*duplicate Issue identified as a duplicate of another issue(s) extensions Issues concerning extensions

Comments

@NotWearingPants
Copy link
Contributor

NotWearingPants commented Oct 1, 2018

Extensions should be ran inside a vm2 NodeVM, with a whitelist of builtin node modules that the extension uses, specified in the extension's manifest (package.json). The whitelist should be optional - if it is specified, the extension would not be able to use any other builtin module, and if it isn't specified, the extension would be able to use any builtin module.

Reasoning:

I download and run VSCode because I trust that it's not some malicious program, and also that it does what it says it does.
But extensions are third party code that I download and run, that might be malicious, or do things I don't expect or that aren't specified - like collecting telemetry without my consent, or placing random config files on my desktop.

For example, VSCode has a telemetry.enableTelemetry setting, but it is not enforced.
The python extension explicitly specifies in its readme:

This extension respects the telemetry.enableTelemetry setting

I haven't seen that statement in all other extensions that I use, does that mean that they collect telemetry? I don't have any way to find out.

Many users will choose to not install extensions if they fear that it might send telemetry.
Also installing an extension that doesn't have many installs is risky, because it might be malicious (I'm not aware of any auditing that an extension publisher has to go through to publish an extension).

Since VSCode relies a lot on extensions to deliver functionality, it should strive to keep users willing to install extensions, and not afraid to.

A (probably) bad solution:

A possible solution would be to run extensions as regular JS without node's builtin modules, and provide functionality with a custom API for filesystem access, for network access, etc.

But this means that extensions would not be able to use many packages on npm as they are not aware of the custom API provided by vscode, and they will try to require('fs') or something.

My solution:

I suggest running all extensions inside a vm2 NodeVM, with no restrictions by default, but with an optional whitelist of builtin node modules that can be specified in an extension's manifest.

The module whitelist would be displayed in the extension's page in the marketplace, and would give users the opportunity to judge for themselves if they want to install this extension on their computer.
Extensions shouldn't have to change their behavior, only to list what modules they are using exactly.

Also for extensions that don't enable the restriction it can be helpful to display the modules the extension uses at runtime, so that you can judge what it's doing after enabling it (better late than never). This could also help extension authors to figure out exactly what they should put in their whitelist. Note that this might require adding a feature to vm2, but this isn't so necessary.

The vm2 package allows running untrusted code in a separate execution context, with a guarantee that that code cannot escape the context it is in. From the readme:

vm2 is a sandbox that can run untrusted code with whitelisted Node's built-in modules. Securely!

(It also isn't an esoteric package, it currently has 173k monthly downloads, and 131 dependents on npm)

Also note that the whitelist will need to include native .node modules, but I'm not sure how many extensions even use a native node module, so this might not be necessary.

Extra thoughts:

vm2 also supports giving mock implementations to some builtin functions, which could allow restricting filesystem access to a certain root directory, or disabling the network for an extension when the user wants to disable telemetry.

A possible problem with the whitelist is that if an extension whitelists the process or child_process modules it can run a native application and bypass restrictions, but that's okay as it would be listed in the whitelist in its marketplace page, and let users will decide themselves. The whole idea is to give more power to the user to help him/her decide whether to install the extension or not, so any information is better than none, and hopefully not many extensions use these modules.

Why would extension authors use this?

This gives extension authors the possibility of being more transparent about what their code does.
If major extensions enable the restrictions on themselves (and possibly also see an increase in users which previously didn't want to install this extension as they didn't know what it could do), the other extension authors will follow.

In the future, this might also result in the community refusing to install extensions that don't specify a whitelist (or that use child_process or something), which gives more power to the users.

Also if in the future over 95% of the extensions have it enabled, it could be worth considering it a requirement to publish extensions.

@vscodebot vscodebot bot added the extensions Issues concerning extensions label Oct 1, 2018
@RMacfarlane
Copy link
Contributor

Closing as a duplicate of #52116

@RMacfarlane RMacfarlane added the *duplicate Issue identified as a duplicate of another issue(s) label Oct 2, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*duplicate Issue identified as a duplicate of another issue(s) extensions Issues concerning extensions
Projects
None yet
Development

No branches or pull requests

2 participants