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

New permission to run Deno programs to avoid --allow-run privilege escalation #2128

Open
rsp opened this issue Apr 16, 2019 · 2 comments
Open
Labels
permissions related to --allow-* flags suggestion suggestions for new features (yet to be agreed)

Comments

@rsp
Copy link
Contributor

rsp commented Apr 16, 2019

Currently the --allow-run is a carte blanche privilege.

Running a script with --allow-run we might run it with --allow-all just as well, because a script having --allow-run can run itself with --allow-all and escalate its Deno privileges (not the OS privileges).

We need a new permission type and a command similar to Deno.run() to run other Deno programs only (e.g. Deno.denoRun() or a different name) but with additional property in RunOptions being permissions to keep. Running:

Deno.denoRun('abc', { permissions: { read: true } });

would run the same deno interpreter with permission arguments being the intersection of permissions in Deno.denoRun() and the current Deno.permissions(), e.g.:

  • /path/to/deno --allow-read abc (if Deno.permissions() include read)
  • /path/to/deno abc (if Deno.permissions() don't include read) or alternatively throw an exception (reject the promise) if the required permissions are not present in Deno.permissions() (maybe running with less permissions or throwing can be configurable with some other option)

Current permissions

Currently the permissions are binary, the command line flags take no arguments and the properties of Deno.permissions() are booleans:

$ deno --allow-read <(echo 'console.log(Deno.permissions())')
{ read: true, write: false, net: false, env: false, run: false }

With binary permissions the new Deno.denoRun() would take a simple set intersection.

Future permissions

In the future I expect the flags to take arguments and the properties of Deno.permissions() to be either booleans for absolute permissions (like read everything) or arrays of path names (or network masks for net permission etc):

$ deno --allow-read=/etc/x --allow-read=/etc/y <(echo 'console.log(Deno.permissions())')
{ read: ["/etc/x", "/etc/y"], write: false, net: false, env: false, run: false }

With scoped permissions the new Deno.denoRun() would need to take a subset of the scope from Deno.permissions() and Deno.denoRun() invocation, e.g.

  • Deno.permissions() returns { read: ["/etc/x", "/etc/y"], write: false }
  • Deno.denoRun() requires { read: ["/etc/x/abc"] }
  • script is run with --allow-read=/etc/x/abc

or:

  • Deno.permissions() returns { read: ["/etc/x", "/home/y"], write: false }
  • Deno.denoRun() requires { read: ["/home"] }
  • script is run with --allow-read=/home/y

Possible flag names:

Keeping the existing --allow-run:

--allow-run     Allow running subprocesses (risk of privilege escalation)
--allow-deno    Allow running Deno subprocesses (same permissions or less)

or changing the current --allow-run to --allow-exe or --allow-exec to make the --allow-run in line with deno run:

--allow-exe     Allow running subprocesses (risk of privilege escalation)
--allow-run     Allow running Deno subprocesses (same permissions or less)

Optionally no flag for that functionality

One might argue that running other Deno programs with the same or less privileges should not need a separate permission because doing so the script doesn't gain any more access than it already had, but I'd argue that it might be useful to have a separate permission flag for that to avoid possible problems in the future (like e.g. fork bombs etc).

More info:

See the comments to Deno eval #2081 where I came up with the idea and showed more examples.

Related issues:

@zekth
Copy link
Contributor

zekth commented Apr 16, 2019

This case has also been discussed about allow-net privelege too which would be great to add.

@kitsonk kitsonk added permissions related to --allow-* flags suggestion suggestions for new features (yet to be agreed) labels Nov 5, 2020
@crowlKats
Copy link
Member

crowlKats commented Jul 22, 2021

This is somewhat covered by workers. not a perfect solution, but it does the job a large amount of the time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
permissions related to --allow-* flags suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

4 participants