On Wed, Jun 02, 2021 at 07:36:31PM +0200, Jordi Vilar wrote: > > I am really wary about the security implications of this. Most obviously > > with the idea of allowing to _override_ commands. Take for example > > `git-lfs`: the assumption is that it is safe for Git to call `git-lfs` > > after the initial check-out phase, but with this feature, it would be > > possible for Git to clone a malicious repository and _immediately_ > > executing code it just cloned, _without_ giving the user a chance to even > > inspect the code. > > You are again right. That's why I was suggesting the conservative > approach of not prepending to the default search path, but appending > to it, so there is no chance of overrinding existing tools. To me, this does not appear to be a conservative approach as you suggest. The only difference between exporting PATH=$SEARCH_PATH:$PATH and PATH=$PATH:$SEARCH_PATH is that the former allows overwriting the results of looking up a binary in the path, but the latter lets you resolve locations that $PATH alone would not find. Suppose you maliciously included a git-lfs binary with your repository. If you include that binary in your PATH ahead of the existing system PATH, then you'll replace system git-lfs with your malicious one, which I think you and I both agree is bad. But if you instead append the malicious binary onto the right-hand side of your PATH, then you can't overwrite a git-lfs already on the path, but you *can* trick a system which doesn't have a version of git-lfs installed into thinking that one exists. So your exploit would just be limited to having someone clone your repository who doesn't already have git-lfs installed into their path, which I would argue is just as bad. > Also, config is not versioned, so, right after cloning you wouldn't > have this option enabled, so you are always safe after cloning [...] I know that this has come up in some recent-ish discussions, and I have not been convinced that this makes things any safer. Thanks, Taylor
