postmodern / chruby

Changes the current Ruby
MIT License
2.88k stars 189 forks source link

Add safe local bin directory to PATH #467

Closed svoop closed 2 years ago

svoop commented 2 years ago

This feature request is for those who create binstubs manually with bundle binstubs --all or automatically (on installs) with bundle config bin bin. Here's an example:

bundle binstubs --all   # creates bin/ directory
bin/rake                # sames as "bundle exec rake"

There's a nifty trick to make them available without the bin/ prefix – which does away with accidentally falling back to the wrong wrapper in case you forget the bin/ prefix:

export PATH=".bin_path/../bin:$PATH"
mkdir .bin_path

By adding this little .bin_path detour to the PATH, all executables in the local bin/ directory are only really in your PATH if this empty .bin_path directory is present. Normally, you'd extend the PATH in ~/.zshrc or similar.

Unfortunately, this doesn't work with chruby/chruby.sh which adds the bin paths for the current Ruby to the beginning of the PATH and therefore in front of this above detour.

Would you consider to merge a PR which extends chruby/chruby.sh to always add .bin_path/../bin as the very first segment added to PATH – along with a description of this trick in the README?

Or maybe you see another solution to make this work?

postmodern commented 2 years ago
  1. Adding an arbitrary local directory to the beginning of PATH is a security risk. If you happen to install a malicious project which installs a malicious binstub for say sudo that steals your password, and you add the binstubs directory to the beginning of PATH, the malicious sudo binstub file will shadow and override the legitimate /usr/bin/sudo.
  2. This could be handled by another shell script to add a binstubs directory to PATH and seems a bit out of scope for chruby, which should only change the current ruby.
  3. I am not really a fan of binstubs, and think that rubygems (which installs it's own binstubs into $gem_dir/bin) should handle automatically selecting the desired gem version based on the Gemfile.lock; I believe there were plans to implement that? Also, I am not really a fan of how binstubs are installed into bin/, which used to be the canonical directory where you put a gem's executable commands into, and how bundler decided to move all executable files into exe to free up bin. Binstubs really feel like a stop-gap solution to typing bundle exec all the time that needs to be rethought.
  4. In the future, I want chruby-1.0.0 to allow overriding certain internal functions which set/reset PATH so you could add additional paths to PATH yourself.
svoop commented 2 years ago
  1. Adding an arbitrary local directory to the beginning of PATH is a security risk. If you happen to install a malicious project which installs a malicious binstub for say sudo that steals your password, and you add the binstubs directory to the beginning of PATH, the malicious sudo binstub file will shadow and override the legitimate /usr/bin/sudo.

Agreed, that's why the .bin_path directory has to be created as a safety to enable the local bin directory only in certain places.

Isn't the same true for the bin directories chruby currently adds to the beginning of the $PATH such as /home/myuser/.gem/ruby/3.0.3/bin? A malicious gem can provide a sudo executable and chruby would already expose it... or do I miss something here?

  1. I am not really a fan of binstubs (...)

Totally agree, but I'm also sick of bundle exec – and even more so forgetting to type it. Maybe, Bundler features a better approach at some point, but I might be retired before that. :smile:

  1. In the future, I want chruby-1.0.0 to allow overriding certain internal functions which set/reset PATH so you could add additional paths to PATH yourself.

This sounds awesome!

postmodern commented 2 years ago

Agreed, that's why the .bin_path directory has to be created as a safety to enable the local bin directory only in certain places.

and what if the attacker also put a .bin_path directory in their malicious git repo ;)

With chruby ruby directories you install them via ruby-install/ruby-build, which only downloads ruby's from the official cache.ruby-lang.org and checksums them. There is a higher chance of malicious binstubs sneaking into a random git repository you decided to clone and checkout, than a ruby you installed. You could also potentially install a malicious gem, but it would be more subtle to use binstubs in a malicious git repository to temporarily get a malicious executable into PATH; a malicious gem would probably get discovered with which sudo since it would be present 100% of the time.

Totally agree, but I'm also sick of bundle exec – and even more so forgetting to type it. Maybe, Bundler features a better approach at some point, but I might be retired before that.

In the meantime, I started aliasing common ruby commands to a shell function that checks for Gemfile.lock and optionally executes the command via bundle exec or command. Despite having to explicitly alias specific commands, it just works.

svoop commented 2 years ago

and what if the attacker also put a .bin_path directory in their malicious git repo

Fair point, some people use .git/safe/../../bin to make this harder. But that's yet another assumption. I'll drop this idea and fall back to my previous solution in .zshrc – not elegant but does the job:

for command in rake rails hanami middleman; do
  alias $command="bundle exec $command"
done

Thanks for explaining and for chruby!