Re: proposal for additional search path in config

On Wed, Jun 02, 2021 at 07:36:31PM +0200, Jordi Vilar wrote:

quoted

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