Re: proposal for additional search path in config

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 3, 2021, 02:45 UTC
Message-ID: <YLhCPMMhD9F3qH/l@nand.local>
In-Reply-To: <CAE-zgtZroyEwG1k9y-XXAx2NKPF=Lav4YG+f7mF227FEeuxDVw@mail.gmail.com>

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

Show 11 quoted lines

> > 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