Sandbox mode for external scripts

[GerbenRampaart]

Sorry if this has been asked or discussed before. Can't seem to find it.

Can I make sure imported scripts are in Sandbox mode even if my own script isn't.

// I'd like to make sure this imported script doesn't have access to disk or network even though my code is networking
import { SomeFeature} from "https://url.com/some-small-library.ts";

[Macil]

I don't think this is implementable in a safe way without lots of work and unstandard restrictions. Javascript modules still run in the same global context and can easily modify global variables to steal capabilities from other modules. See #1189 (comment) and #378 (comment) for further explanation.

[hayd]

Something, that's could go along way her, is to support a kind of CSP... so that e.g. you could restrict (via a whiteless) which domains deno could make network requests to, or where on the filesystem it can access, or which env variables. Rather than these being simply on/off.

[GerbenRampaart]

I feel this is still a compelling feature, given the amount of node modules people just install and run in production without ever evaluating the source. See https://hackernoon.com/im-harvesting-credit-card-numbers-and-passwords-from-your-site-here-s-how-9a8cb347c5b5.

Reading @Macil comments in previous threads seems this has been considered extensively enough. For various reasons it's hard to implement.

[paulmillr]

Yeah, I think that's a must-have. A common way of using dependencies is having your own application code use all privileges while third-party deps may just help in a non-sandboxed way.

That's the main issue with node.js dependencies: any dep, no matter how small, could leak your shit to thieves. If we had a way to ensure most third-party code cannot execute any privileged code, that'd be perfect.

[hayd]

Having a CSP which restricts on domain + filename + env etc. would be a huge win. Ideally you'd be able to configure based on source location, but if that's infeasible (which I am not convinced) doing at the top-level still mitigates a bunch of attacks.

[paulmillr]

I don't really see any point in sandboxing if people would still be able to backdoor their packages similar to event-stream issue

[hayd]

The attack described above would be completely mitigated if you - at the top-level - restricted domains (specifically don't allow POST requests to evil.com... unless it's in the whitelist). Similarly if you could additionally configure based on source location.

[kitsonk]

There is a big difference between sandboxing remote modules and limiting the scope of network access. I think the latter is more logical. Treating remote modules differently is going to be really stupidly hard. Where as restricting external connectivity is different.

We should treat programmes as a whole, whether the be local or remote modules. Having the ability to support a whitelist and blacklist modes for both network and file access would solve most of the exploits that are possible. If the workload needs a blacklist approach, then the author needs to have better controls in place for all their code, local and remote.

[paulmillr]

Exactly what do you mean by better controls? Do you know, how the event-stream trojan was executed? It changed JS prototypes.
If you’re suggesting everyone should audit what they’re depending on, that’s cool — But feels like wishful thinking. Not going to happen. Folks are not going to audit 500 dependencies for every project.

[paulmillr]

It actually amazes me how this and similar issues have been solved a long time ago in Haskell etc. Very simple: there’s IO monad. If your method returns IO, there would be IO. If it doesn’t, there won’t be.

[ry]

btw the idea behind deno_std is to audit a large body of code so that transitive deps largely terminate in the same trusted code base.
There is no avoiding the need to audit code you depend on.

[paulmillr]

@ry great idea!

[hayd]

@paulmillr the attack above worked because node allowed network requests to evil.com (I forget the actual domain), if it's a whitelist and deno can't make requests to arbitrary urls that mitigates that (large) attack vector.

[paulmillr]

@hayd is there a whitelist planned? That could help a lot.

[kitsonk]

Exactly what do you mean by better controls? Do you know, how the event-stream trojan was executed? It changed JS prototypes.

Better controls, as in far of source and security of their code. External concerns that Deno alone cannot solve. We assume a remote module is less trustworthy than local code. That is really a false assumption because in reality, single developers seldom are the only people with access to local code. Exploits are obfuscated. A bad actor with access to the local code is much more likely to exploit, as there are far more eyes on a distributed exploit (which was how the recent attack was detected).

@hayd is there a whitelist planned? That could help a lot.

@paulmillr that is what I was proposing, a whitelist and a blacklist, versus sandboxing. Certain workload means you take certain risks. Deno can only go so far in helping.

[hayd]

Also whitelist for filesystem and env access (maybe subprocess too?). This would be relatively straightforward by amending the DenoFlag&IsolateState to include regex/strings (?)... not sure if would affect perf / need a cheaper way.

I do think additional sandboxing is worth exploring... E.g. I only want a particular lib to access my aws env variables. (Whether there's a solution which can't be sidestepped...)

[kitsonk]

Sandboxing is just a mess, and I don't think it would deliver any real world security. There is now way to make it make sense from a sensible default, so that means people wouldn't be bothered.

Outside of the whitelist capability for net and file system, the only other attack vector to narrow down would be to whitelist modules that can load the deno module. If the module isn't on the whitelist, then throw in Rust. While it might not be perfect, with modules loading other trusted modules that would leak deno APIs, it does dramatically increase the "behavioural" security analysis of the module.

That leads to another possibility, an "audit" API, where all privileged operations are available to be sent somewhere for analysis or threat/exploit detection.

[hayd]

Having a log of all privileged ops seems like a good idea. 👍

Maybe I'm unclear what precisely "sandboxing" means here...

If you can "whitelist modules that can load the deno module" why not be able to impose some other restriction e.g. a smaller domain whitelist. ?

There's definitely the problem of an API for something like that, but it sounds like the underlying machinery could be feasible...?

[Macil]

(Just to be clear, in this post I'm using "sandboxing" to refer to the general concept of running code with fewer permissions.)

Any sandboxing controls (including limiting access to the deno module) that apply on a finer grain than a Javascript context/realm (such as on modules) will usually be bypassable with attacks like those in #1189 (comment).

Currently, deno does sandboxing on the process level (the whole process has filesystem access allowed or disallowed based on command line flags, etc). There is a step further that deno could go easily: deno could allow Workers to be created that have more restricted permissions. It could look like this:

const smallLibWorker = new Worker("https://url.com/some-small-library-worker.ts", {denoPermissions: ["allow-read"]});
smallLibWorker.postMessage(123);
smallLibWorker.onmessage = message => {
  console.log('got reply from smallLibWorker', message);
};

That's not exactly sandboxing for arbitrary imports, but it's close to the OP request.

[kitsonk]

Just to be clear, what is my mind when we say "sandboxing" is that it runs in its own environment, specifically a different instance of the global scope, with limited or no communication to other environments.

The OP asked for it for remote modules. That is impractical, would make a lot of use cases unworkable, and generally not significantly improve security.

Constraining privileged APIs, but all modules running in the same environment, I wouldn't consider sandboxing.

Web workers inherently are a form of sandboxing. I think the idea of providing Deno security flags on Worker init is a great way for people to explicitly sandbox under a different set of controls! I really like it. We can argue the semantics of the options, but it is a really good idea!!! I checked and excess properties in the options do not cause browsers to though, so code portability should be ok.

[hayd]

Constraining privileged APIs, but all modules running in the same environment, I wouldn't consider sandboxing.

what would this approach be called?

[kitsonk]

what would this approach be called?

It is the existing Deno security module, just finer grained... permissions based security.

[hayd]

"but all modules running in the same environment" does this also mean sharing the same constraints?

[paulmillr]

The OP asked for it for remote modules. That is impractical, would make a lot of use cases unworkable, and generally not significantly improve security.

Can you clarify why exactly it won't improve sec? I'm pretty sure sandboxing single modules would improve security a lot.

[kitsonk]

Can you clarify why exactly it won't improve sec? I'm pretty sure sandboxing single modules would improve security a lot.

How? Outside of breaking lots of code. I am afraid I can't prove a negative. If you can explain how sandboxing each module would increase security. The initial runtime is a sandbox of its own. It is a totally fallacy to trust a local module more than a remote module.

"but all modules running in the same environment" does this also mean sharing the same constraints?

Yes, because I don't think varying the constraints for "this program" is logical... if there is something a local module shouldn't do, then a remote module shouldn't do it... if there is something a local module should do, a remote module should do it... couple that with any realistic implementation of sandboxing would mean breaking pretty much everything and putting on a huge overhead. Essentially every module would need to be a seperate web worker, dealing with all the limitations that come from postMessage. There would be no sharing of references/symbols/etc. across the sandboxes.

[paulmillr]

Huh?

1. You have an app that has the access to the outside world for some of its functionality. For example, it fetches updated signatures or something - once in a while. So it needs network access. But only for one particular module.
2. Your app at the same point has 400 dependencies, which you won't audit. No one would really audit that besides a few percent of users.
3. A hacker gains access to one small dep of 400 deps. The code on NPM doesn't match GitHub — and it's not easily verifiable at all. Once in a while, the module uploads your server files, configs and creds; and stuff like that. Not easily noticeable.
4. You're screwed.

If all modules except for 2 don't have any access to outside world - and they SHOULD NOT - the problem would be easily mitigated. That's a real threat today - while "sandboxing an entire app" is useful only for microservices; it adds no value for monoliths.

As for the implementation itself - i'm positive proper sandboxing can be achieved without huge performance downgrade. Think creatively.

[kitsonk]

1. You have an app that has the access to the outside world for some of its functionality. For example, it fetches updated signatures or something - once in a while. So it needs network access. But only for one particular module.

Granular permissions address this. A white list of the one valid network destination.

2. Your app at the same point has 400 dependencies, which you won't audit. No one would really audit that besides a few percent of users.

If your app has 400 modules, what is the chances the only permission you need is one narrow slice of privileged access? You are simply going to --allow-all because you are a lazy developer who doesn't care about their dependencies. I am talking about real world security. Not giving someone keys and allow them to prop open the door because it is too hard to give keys to all the right people for proper access. Therefore sanboxing buys you nothing. Sandboxing is the wrong solution for grainular permissions.

If you have a security need and you can deal with the limitations of sandboxing, load all your external modules in a web worker. As suggested above, varying the Deno permissions for that web worker is an ideal way. That is real world security.

As for the implementation itself - i'm positive proper sandboxing can be achieved without huge performance downgrade. Think creatively.

I guess I can't. Sandboxing 400 modules would break lots of portable code and dramatically impact the memory footprint.

[kitsonk]

Actually, another side conversation reminded me... We could/should support realms: https://github.com/tc39/proposal-realms

That would be as efficient version of sandboxing without using web workers. It is still arguable how automatic it would be.

[hayd]

It needn't break any code, this could be completely opt-in. Choosing a backwards compatible API is achievable... Something like:

// deps.ts

@trust({env: "^AWS", net: "^[a-z\.]+\.us-west-2.amazonaws.com"})  // requires parent to have this or higher permission
import * as aws from "XXX/aws-sdk/mod.ts";
export aws;

@trust()  // label to say "no trust" (perhaps optionally can make this the default)
import * as es from "XXX/event-stream/mod.ts";
export es;

The problem isn't the API, it's feasibility... but it seems like this could (eventually!) be done with Realms. 👍 Almost certainly it won't be perfectly secure, but it'd cut off another bunch of attack vectors.

[paulmillr]

@kitsonk I don't care whether it's realms, whitelist, or sandbox. That's just a matter of implementation.

What I argue here for is any way to protect production systems and developer machines from modules gone rogue. If we can achieve that, and the result is simple in development experience terms, that would be ideal. If it's painful for developers to use, the result would be the same as if there was no such functionality.

[paulmillr]

I'm arguing for any sort of sandboxing — whitelisting stuff is also sandboxing. It's just different semantics.

[hayd]

expression level syntax

The syntax is not important, and way less important that the implementation. The semantics above are that each labelled dependency has its own Realm (any dependencies of that module would inherit that Realm). Breaking code is a feature (and you may be right there could be serialisation issues) and that's why it would have to be opt-in...

Then why are you trying to argue for sandboxing? I suggested several other approaches...

We want all the things. 😉

[Macil]

@kitsonk I don't care whether it's realms, whitelist, or sandbox. That's just a matter of implementation.

Realms, workers, or other similar safe sandboxing mechanisms are all incompatible with standard import statements. They each require code to be written tailored for them. They can't be an implementation detail. I think this thread is having some miscommunication and essentially going back and forth between "sandboxing is important, let's sprinkle it on to imports" "that can't work out on imports, we can do something similar" "but we should do sandboxing".

[hayd]

I think there's two actionable things here:

1. Add string/regex options for Permissions (to tighten net/env/file/sub access).
2. Allow restricting Worker permissions (to more restricted subset than process that created Worker).
3. Not yet actionable. Realms (something to think about in the future / once it's stabilized).

[afinch7]

Just submitted #1864 should make option 2 pretty easy to implement.
I like that approach alot it would satisfy many of my needs, and do it in a way that is inline with most of the deno design.

[natevw]

Not directly applicable, but a team within Google recently open sourced a sandboxing framework that wraps C libraries:

From a high-level perspective, Sandboxed API separates the library to be sandboxed and its callers into two separate OS processes: the host binary and the sandboxee. Actual library calls are then marshalled by an API object on the host side and send via interprocess communication to the sandboxee where an RPC stub unmarshals and forwards calls to the original library.

— https://security.googleblog.com/2019/03/open-sourcing-sandboxed-api.html

So IIUC basically what deno does at the process level between the JS code and the OS, this makes happen around a given set of extern "C" function calls. It looks like the calling code is significantly affected for pointer handling and other complexities left exposed.

[rsp]

If only Deno was not a global, then all of the above would be possible with ease.

Imagine that there is no Deno object, but a getDeno() function that works only once.

In your own program at the very beginning you would do:

const Deno = getDeno();

But the libraries would know that they cannot rely on getDeno() so they need to get the Deno object from the code that use them using simple dependency injection.

So, as a library author instead of writing:

export function a() {
    Deno.x();
}

export function b() {
    Deno.y();
}

you would write, e.g.

let Deno;
export function useDeno(deno: Deno) {
    Deno = deno;
}
// ... the rest

and use it as:

import { a, b, useDeno } from './mod.ts';
useDeno(Deno);

or:

export function mod(Deno: Deno) {
    return {
        a() {
            Deno.x();
        },
        b() {
            Deno.y();
        },
    };
}

and use it as:

import { mod } from './mod.ts';
const { a, b } = mod(Deno);

or you could use few other techniques. But the point is that if the modules had no other option to get the Deno object than having it injected by the caller then you would have a good control of which libraries use Deno in the first place, so you would not give it to a regex validation library or something that should not use the system resources.

But even better, for any module that needs access to the system, you could give a wrapped version of Deno with only the methods that are needed, possibly with additional parameter validation.

Because I think that @paulmillr is arguing that if you have a complex application that stores sensitive files and needs to use some logs then you don't want the logging library to have access to anything else than a single directory where it needs to save logs, and if you have a number formatting library then you don't want it to have access to anything. Or the example by @hayd with not giving AWS credentials in the env vars to every dependency and subdependency is also very reasonable.

But I also understand why @kitsonk is saying that adding a new syntax or entirely new sandboxing to all imports would be a mess, and it's true what @Macil is saying about some of the things described in this issue as being incompatible with the normal imports.

Not having a Deno global object on the other hand would mean that we would only pass it to something that needs it with no new import syntax, no module sanboxing, no per-module privilege configuration etc.

Of course the problem is that Deno is a global variable and because of that everything has access to everything. If there was a command-line switch to turn off Deno (like with noDenoNamespace for workers in #2717) then most of the things described in the comments above would be easy to implement with changes in Deno itself.

(Of course it would be possible for one module that gets the Deno object to pass it to another one, but it will be at least possible to control which library gets access to what. Currently everything has full access to everything so any restriction of that could only be an improvement.)

A switch like this being optional would not break any existing code. Some security-conscious libraries could require support it (not require because having an object that is not used would not break anything), most would probably not. But the beauty is that all of the libraries that don't need the Deno object would not care whether or not it is available so they would work the same in both modes with and without the global Deno object - with the only exception that if they ever get compromised then they would crash trying to use Deno instead of causing any harm (when they were executed with a --safe-deno switch or whatever it would be called).

In fact the only thing needed for a proof of concept would be a way to remove Deno from the global object. @kevinkassimo do you think that making Deno not deletable/writable as you did in #2153 could be the default but optional, with a command line-switch that disables the freeze?

I agree with @ry saying that There is no avoiding the need to audit code you depend on but if every code that we depend on (and the code that this code depends on, etc.) was 100% proved to never make a mistake of writing to a wrong directory then we wouldn't need --allow-write and other permissions that much. I suppose that not everyone will be skilled enough to audit hundreds of thousands of lines of code and be absolutely sure that there is no path traversal vulnerability anywhere or any other potential problem. I'm not saying about obvious trojans but honest mistakes. It's not that easy to spot some of them.

Having an additional layer of safety by (optionally) not exposing the Deno object to every dependency would be a nice feature that is not hard to implement (just a way to delete the global Deno would be enough).

Edit: I think that by not having a global Deno most of the features described by @rivertam in Permissions on a module-level #171 could be done with no changes to the import syntax needed. Of course it might not be as convenient, but it will all be possible (please @rivertam correct me if I'm wrong).

[Macil]

@rsp Removing the Deno object from the globals alone isn't secure because modules can redefine global functions in order to capture the Deno object.

For example, if you had a privileged module use a privileged API in a callback that's passed to a global function, like list.map(fileName => Deno.readFileSync(fileName)), then a different module (which was intended to be sandboxed and never had Deno passed into it) could redefine Array.prototype.map to capture the callback and then the module can gain access to Deno.readFileSync.

Removing Deno from globals could make modules easier to manually audit, but it seems like a big change if it doesn't make things secure, and it might give some users a false sense of security.

There is a Javascript standard proposal for allowing this pattern to be secure though: https://github.com/tc39/proposal-ses. The idea is that you could make a Frozen Realm where all of the global variables are fully immutable, and then you could execute untrusted code within that realm. Code within the frozen realm wouldn't be able to redefine global variables to try to capture capabilities (the Deno object), and therefore could only use capabilities if passed to it.

[kevinkassimo]

@rsp Haven't fully gone through your reply, sorry if I missed anything.
Just want to mention that we do have noDenoNamespace for worker options. And based on current design, it is also possible to hide Deno namespace in the main worker (through some little tweak). About getDeno() this is fully doable (reference to Deno and Deno.core is always available in internal code. We just hide the reference from user script). Also we can possibly make it prompt Worker requests Deno namespace. Grant?.

I still believe making Deno frozen is probably the right thing to do in normal usages, but as I said above, we can add an option --no-deno-namespace that simply get rid window.Deno in the main worker.

[kevinkassimo]

IMO a pattern I think might be acceptable is to make main worker having access to Deno namespace, while untrusted scripts would be placed in a worker without Deno namespace. If it requests any privileged operations, it should send a message to the main worker and delegate the work, and simply get the result back in postMessage from main worker. (once transferring algorithm (especially on ArrayBuffers) between workers is correctly implemented this should have minimal cost)

I would even imagine a library developed to hide all these details: a "polyfill" for Deno namespace that only implements async ops, and also adding an extra listener to onmessage. On each e.g. Deno.readFile(...) it would internally create a promise, sending message to the main worker, and once response posted from main worker is received, resolving the promise with the expected output.

Some similar work I have seen before: https://github.com/ampproject/worker-dom this library implements many DOM APIs through messaging to main worker.

[rsp]

I still believe making Deno frozen is probably the right thing to do in normal usages, but as I said above, we can add an option --no-deno-namespace that simply get rid window.Deno in the main worker.

@kevinkassimo, If there was an option like this, with some way to get the Deno object once (the getDeno() idea) then this would be enough for pretty much everything that I described.

IMO a pattern I think might be acceptable is to make main worker having access to Deno namespace, while untrusted scripts would be placed in a worker without Deno namespace.

That would be the most secure option but it would mean that using something like lodash running in a different worker would be not as convenient.

But it's a nice idea of having a Deno-like interface for inter-worker communication. This can also be used independently of disabling the window.Deno with --no-deno-namespace.

I am not arguing that the pattern that I am proposing is the only or the best way to solve all problems, I am just saying that it is one way with a relatively simple solution (e.g. compared to changing the syntax of imports and convincing TC39 to adopt it etc.)

[rsp]

For example, if you had a privileged module use a privileged API in a callback that's passed to a global function, like list.map(fileName => Deno.readFileSync(fileName)), then a different module (which was intended to be sandboxed and never had Deno passed into it) could redefine Array.prototype.map to capture the callback and then the module can gain access to Deno.readFileSync.

@Macil , In other words, the worst case scenario would be to have what we got today, where every module has access to Deno.readFileSync with no need for a coincidence of using a function like that in some other part of the code and having to exploit that without breaking all other code that uses [].map().

By the way, I think that by default all built in prototypes should be frozen.

[rsp]

@Macil, to answer your edit:

There is a Javascript standard proposal for allowing this pattern to be secure though: https://github.com/tc39/proposal-ses. The idea is that you could make a Frozen Realm where all of the global variables are fully immutable, and then you could execute untrusted code within that realm. Code within the frozen realm wouldn't be able to redefine global variables to try to capture capabilities (the Deno object), and therefore could only use capabilities if passed to it.

Yes, this is what would be perfect but at least being able to restrict Deno would be a step in the right direction. The frozen realms were discussed in #378 and I agree with what you wrote there.

[kitsonk]

@bartlomieju and I talked last night on another topic which seems to relate somewhat to this. We are going to build a worker in Deno that is only compliant with the worker standard and does not include the other globals. This would be a sandbox of which certain workloads could work in. There will also be an option to spin up a worker with the Deno namespace as well.

Of course as V8 would implement SES, Deno would also integrate it as well.

[bodinsamuel]

Hi team,

I was looking to create an issue about this, but this issue seems the right spot to put more use case.
Right now in NodeJS we have no way to execute untrusted source code. Fortunately someone built a package that is covering all the requirements but it's super hacky and could be really a game changer with deno.

isolated-vm
https://github.com/laverdet/isolated-vm

You probably already saw that, if I'm not mistaken it's currently used by Cloudflare for their own "lambdas" (workers) and we currently used it in Algolia for our Crawler custom codes.

It helps letting the user code a function with anything they want inside and execute it on our servers while not being afraid of leaking info or break our infra.

Right now it's a bit cumbersome, the only way to use it correctly when providing a base of code, is to compile your whole script, then compile your untrusted script and then send both to a new instance of isolated-vm each time.
It has a huge overhead (and also means webpack or something similar).
But after that it's the most awesome thing. You are sure that the code is not modifying any built-in libs, only access the data we passed and even it try to escape the sandbox it will be killed either by CPU limit, RAM limit or time limit.

Why Deno could be a good fit?
It has built-in security and already covers some important restriction of fs, network etc...
But it's not possible to ensure that the untrusted code won't be able to import other files, modify the runtime constant, access env variables, control the cpu, time and ram usage.

I believe it could be part of your roadmap to add more flags to control the execution of scripts even though you are not aiming at full sandboxing. Like e.g: --allow-modify-builtin, --allow-import, --cpu-max=10% --ram-max=512, --execution-time-max=10s it would allow more "graceful exit" and allow us to have better trust than what nodejs is currently proposing.

Example

# Pseudo code example

const myUntrustedCode = `a ts file in a string format`;

const p = Deno.run({
  args: [
    "deno",
    "run",
    "--allow-read=false",
    "--alow-write=false",
    "--allow-net=false",
    "--allow-env=false",
    "--allow-import=false",
    "--allow-modify-builtin=false",
    "--cpu-max=10%",
    "--ram-max=512", 
    "--execution-time-max=10s",
    "--import=./my-trusted-base-code.ts",
    myUntrustedCode,
  ],
  stdout: "piped",
  stderr: "piped"
});

Notice that I'm not even talking about sandbox here, having flags is sufficient enough to create a sandbox without having a new api.

This would allow:

• controlling the execution of untrusted code (let's even say replace eval).
• while still being able to pass a base of code that will be reloaded each time (so even if modified it will be loaded fresh for each execution).
• Rely on deno to kill the subprocess if it goes too high in term of CPU or time.

I believe it can benefit a lot of backend developers, even the ones that are not aware that they have security issues.

[buckle2000]

We need this feature to ensure that only a small chunk of code have FFI access. #5566 It is like unsafe in Rust.

Otherwise, any part of your code can call from libc and summon dragons if you want to use FFI.

[kitsonk]

There has been a lot of good discussion here over the years, but at this point there isn't anything where the upside benefit of increasing security would outweigh the technical or usability challenges of such a solution. We have items like package locks and finer grained controls around dynamic imports and worker scripts and work to vary permissions with workers, which is likely the best "sandbox" approach (see: #4687). Based on this I am closing this for now.

[aksoo5]

@ry This is a game changer, don't regret not implementing this