Re: Security of .git/config and .git/hooks

From: Stefan Beller <sbeller@google.com>
Date: Oct 3, 2017, 15:10 UTC
Message-ID: <CAGZ79kacfdgw27MjSMwiZO-sn2EgjeQdsEUoSs=b_K+ekA4rfA@mail.gmail.com>
In-Reply-To: <20171003123239.lisk43a2goxtxkro@sigill.intra.peff.net>

So once upon a time we compared Gits security model with a web browser. A web browser lets you execute 3rd party code (e.g. javascript) and it is supposedly safe to look at malicious sites.

Currently Git only promises to have the clone/fetch operation safe, not the "here is a zip of my whole repo" case, which sounds more like the web browser experience ("here is a site with js, even zipped in transfer"). Tightening the security model of Git towards this seems like a good idea to me.

Show 15 quoted lines

>>  1. Introduce a (configurable) list of "safe" configuration items that
>>     can be set in .git/config and don't respect any others.
>
> A whitelist is obviously safer than a blacklist. Though I also feel like
> some of the options may give an unexpectedly wide attack surface. I.e.,
> I wouldn't be surprised if some innocent-looking option ends up being
> used in a tricky way to gain more access. E.g., submodule config
> pointing to paths outside of the repository.
>
> Do you plan to run in safe-mode all the time? What if safe-mode was a
> lot more extreme, and simply avoided reading repo-level config at all
> (except for check_repository_format(), which should be pretty innocent).
>
> I have a feeling there are some features (like submodules) that would
> simply be broken in safe-mode.

I would think that the essential submodule things would be "safe" to look at. But e.g. submodule.<name>.update = "!rm -rf /" would be not ok, hence the .update configuration would be in the unsafe space.

Any unsafe config option would need to be set outside the actual repository (~/.config/git/<repo-id>/config ?)

Show 25 quoted lines

>
>>  2. But what if I want to set a different pager per-repository?
>>     I think we could do this using configuration "profiles".
>>     My ~/.config/git/profiles/ directory would contain git-style
>>     config files for repositories to include.  Repositories could
>>     then contain
>>
>>       [include]
>>               path = ~/.config/git/profiles/fancy-log-pager
>>
>>     to make use of those settings.  The facility (1) would
>>     special-case this directory to allow it to set "unsafe" settings
>>     since files there are assumed not to be under the control of an
>>     attacker.
>
> You can do something quite similar already:
>
>   git config --global \
>     include.gitdir:/path/to/specific/repo.path
>     .gitconfig-fancy-log-pager
>
> The main difference is that the profile inclusion is done by path rather
> than riding along with the repository directory if it gets moved. In
> practice I doubt that matters much, and I think the security model for
> include.gitdir is a lot simpler.

I am not sure if this works so well for the submodule.<name>.update config (that we want to deprecate anyway, but still)

Show 12 quoted lines

>> For backward compatibility, this facility would be controlled by a
>> global configuration setting.  If that setting is not enabled, then
>> the current, less safe behavior would remain.
>
> Are config and symlinks everything we need to care about? I can't think
> of anything else, but Git really has quite a large attack surface when
> accessing a local repo. Right now the safest thing you can do is "git
> clone --no-local" an untrusted repo and then look only at the clone. Of
> course nobody _actually_ does that, so any "safe" mode seems like it
> would be an improvement. But would claiming to have a "safe" mode
> encourage people to use it to look at risky repositories, exacerbating
> any holes (e.g., exploiting a bug in the index format)? I don't know.

Good point. Though we only care about the case of breaking out and executing untrusted code; most of the index exploits would rather trigger a segfault or infinite lop (in my imagination at least).

>
> -Peff