proposal for additional search path in config

proposal for additional search path in config

[Jordi Vilar]

Hi folks,

I would like to gather feedback about a feature I'm considering about.
Basically, I would like to propose a configuration setting with a path
(possibly relative to the working tree) to look for executables. In
that way, a local clone could setup additional tools/scripts private
for its work tree, maybe also under revision control.

For instance, we could have a setting like:
  git config core.extensions_path "./scripts"
so that whenever git has to resolve an external command also includes
that path in the search, resolving relative paths to the work tree, in
this case, the "/scripts" subdir.

A simple use case could be a custom tool integrated with git, but
local to the repository and thus versioned.

For instance, let's assume that we have some metadata required by a
custom build tool that is stored in commit comments or tags, so we
have a custom script git-project-metadata to handle it:
  git project-metadata query ...

Maybe the format for the metadata evolves, so the script must evolve
also. Having it integrated in this way would allow that each checkout
sees the correct script version, and that script is always invoked as
a regular git command, without having to tweak the environment
everytime.

It is not clear to me whether it's a good idea for this additional
path to have priority over the default search path, as in one hand it
could allow to override the default tools, but in the other hand this
may be a hole for bad things... For my, the conservative approach of
having this additional path as a fallback is ok, so that it only adds
more commands, but doesn't override commands.

Also, this setting could be set either globally or locally, and in
this case, in the conservative approach, the overloading rule is the
different to the normal one: the system search path in the environment
path variable has precedence over the global setting and this over the
local setting, but all apply.

Does it sound reasonable? Is there interest in such feature?

Thanks,

Jordi

Re: proposal for additional search path in config

[Johannes Schindelin]

Hi Jordi,

On Tue, 1 Jun 2021, Jordi Vilar wrote:

> I would like to gather feedback about a feature I'm considering about.
> Basically, I would like to propose a configuration setting with a path
> (possibly relative to the working tree) to look for executables. In
> that way, a local clone could setup additional tools/scripts private
> for its work tree, maybe also under revision control.

That sounds like a Remote Code Execution vulnerability by design in the
making. But let's read on.

> For instance, we could have a setting like:
>   git config core.extensions_path "./scripts"
> so that whenever git has to resolve an external command also includes
> that path in the search, resolving relative paths to the work tree, in
> this case, the "/scripts" subdir.
>
> A simple use case could be a custom tool integrated with git, but
> local to the repository and thus versioned.
>
> For instance, let's assume that we have some metadata required by a
> custom build tool that is stored in commit comments or tags, so we
> have a custom script git-project-metadata to handle it:
>   git project-metadata query ...
>
> Maybe the format for the metadata evolves, so the script must evolve
> also. Having it integrated in this way would allow that each checkout
> sees the correct script version, and that script is always invoked as
> a regular git command, without having to tweak the environment
> everytime.
>
> It is not clear to me whether it's a good idea for this additional
> path to have priority over the default search path, as in one hand it
> could allow to override the default tools, but in the other hand this
> may be a hole for bad things... For my, the conservative approach of
> having this additional path as a fallback is ok, so that it only adds
> more commands, but doesn't override commands.
>
> Also, this setting could be set either globally or locally, and in
> this case, in the conservative approach, the overloading rule is the
> different to the normal one: the system search path in the environment
> path variable has precedence over the global setting and this over the
> local setting, but all apply.
>
> Does it sound reasonable? Is there interest in such feature?

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.

But even if you _append_ that path to the `PATH` variable, unintended
vulnerabilities could be opened. Take for example `git difftool`, which
looks for executables in the path until it finds one that is installed. An
attacker could guess which executables your setup is missing, and squat on
those, overriding your `git difftool` to execute code you did not intend
to be executed.

So putting a security lens before my eyes, I would be quite worried about
such a feature.

Note that the _use case_ you present is a common one, and what I see
projects do is to integrate the configuration into their build process
(carefully vetting any code changes to the build process is _always_ a
good idea, hence this is a lot safer than having Git configure what is run
automatically). And of course, this is already possible right now, it just
delays configuration of those included tools to the time of the first
build, as opposed to the clone time (as suggested above).

Ciao,
Johannes

Re: proposal for additional search path in config

[Jordi Vilar]

Hi Johannes,

Thank you for taking the time to read the proposal and reply with your comments!

> That sounds like a Remote Code Execution vulnerability by design in the
> making. But let's read on.

I agree with you that a too simple implementation of this would pose
an unacceptable risk. But I think the details are important in order
to assess the actual issues. Let me develop those details as I reply
to your comments.

> 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. Also,
config is not versioned, so, right after cloning you wouldn't have
this option enabled, so you are always safe after cloning. Then you
should have to manually configure it, of course, only for trusted
repositories. But if you configure this search path in the global
settings, you should avoid setting a relative path (or maybe git could
ban it). But in any case, only the user can set this, it is not
automatic upon cloning at all.

> But even if you _append_ that path to the `PATH` variable, unintended
> vulnerabilities could be opened. Take for example `git difftool`, which
> looks for executables in the path until it finds one that is installed. An
> attacker could guess which executables your setup is missing, and squat on
> those, overriding your `git difftool` to execute code you did not intend
> to be executed.

Well, the apparent effect would be like appending to the PATH
variable, but I'm not suggesting to implement it in that way. I didn't
say it explicitly, but what I had in mind is that git searches for
extensions also in this additional path, but without modifying the
environment variable, so it is not inherited by any executable or any
other built-in git functionality.

> So putting a security lens before my eyes, I would be quite worried about
> such a feature.

And I appreciate your critical thinking. I just want to share an idea
I think could be useful, not to open a hole that we could later regret
to have open, so all precautions are good.

> Note that the _use case_ you present is a common one, and what I see
> projects do is to integrate the configuration into their build process
> (carefully vetting any code changes to the build process is _always_ a
> good idea, hence this is a lot safer than having Git configure what is run
> automatically). And of course, this is already possible right now, it just
> delays configuration of those included tools to the time of the first
> build, as opposed to the clone time (as suggested above).

For sure, decoupling sources/resources from tools is optimal. But not
always possible. And having to setup the environment for each worktree
makes working with multiple worktrees at a time from the same shell
session a pain and error prone.

Regards,

Jordi

Re: proposal for additional search path in config

[Taylor Blau]

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