Open ivan opened 7 years ago
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.
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)
@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.
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.
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.
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.
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 / @ivan / @drichardson can you check the latest commits and let me know what do you think?
My only suggestion is to not use system time for the filename (https://github.com/tonini/alchemist-server/commit/bbd1d207724cbb5d730c672d9ce2cea6df096a08#commitcomment-23265073).
Otherwise I think this is the correct solution.
LGTM
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
.
alchemist-server starts a server that executes arbitrary code from any host that can reach you on the network, without any kind of authentication.
Take note of the port, because the exploit below doesn't brute-force the port (though this could easily be done by an attacker).
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).