search

JonyEpsilon / gorilla-repl

A rich REPL for Clojure in the notebook style.
http://gorilla-repl.org
MIT License
887 stars 104 forks source link

Gorilla-launched nREPL binds to all addresses #217

Closed Marand- closed 9 years ago

Marand- commented 9 years ago

When Gorilla starts the nREPL server, it's binding to all addresses, allowing untrusted hosts to connect and interact with the REPL. There is an :ip configuration option, but it only limits access to Gorilla, leaving the nREPL open to all. This doesn't happen when running nREPL directly with lein repl :headless, which binds to localhost only.

This seems to be because, until recently, nrepl bound to 0.0.0.0 by default if :bind was not specified. The default's been changed to localhost recently, but lein is still using 0.2.6 so gorilla's still vulnerable.

Gorilla should probably explicitly bind to localhost, or maybe use its :ip option to bind to the same address that the notebook itself is bound to.

JonyEpsilon commented 9 years ago

Thanks for the heads-up (and sorry for the slow reply!). I'll upgrade Gorilla's nREPL dependency in the next release ... although I recall that last time I looked at this there was some difficulty with Leiningen bringing in the old version, will have to check that.

Marand- commented 9 years ago

No problem. If you can't update the nREPL dependency, you can still work around it with a very small change to the gorilla code. I did it as a workaround, it was just a one-line addition to one of the .clj files if I remember right; I'll have to check and make sure when I'm home and have a chance.

I did it based on some info in the nrepl source about binding to IPs and ports, so it should be safe for general use. I'll double check when I can, because trying to deal with it from a tablet would suck.

Marand- commented 9 years ago

Found it. I just added it into the clj file manually and inserted the file back into the jar, so I don't have a proper patch or anything, but in nrepl.clj, lines 16-17 you can add :bind "localhost" and it will force the lein version of nREPL to behave properly without a need to change nREPL versions ahead of lein.

In the local copy I have, it looks like this:

        nr (nrepl-server/start-server :port nrepl-requested-port
                                      :bind "localhost"
                                      :handler (apply nrepl-server/default-handler middleware))

I thought of it after I posted the report but didn't think to suggest it as a fix since nREPL already dealt with it directly. Still, might be useful info later for adding an option to allow the user to configure what IP to bind to. Not sure how likely a use case for that would be, though.

JonyEpsilon commented 9 years ago

Thanks @Marand- , that's very helpful :-) That should be an easy way to fix the problem if the nREPL upgrade is not possible.

JonyEpsilon commented 9 years ago

Have upgraded to latest nREPL on develop which fixes this. Will be in the next release.