Re: [PATCH 00/12] Fix various overly aggressive protections in 2.45.1 and friends

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: May 27, 2024, 19:51 UTC
Message-ID: <99225123-70f0-3546-a6fa-b6d1f981b41d@gmx.de>
In-Reply-To: <Zk9vafYPijqyWpXv@kitenet.net>

Hi Joey,

On Thu, 23 May 2024, Joey Hess wrote:

Show 12 quoted lines

> Junio C Hamano wrote:
> >  - The extra check seems to have meant to target the symbolic links
> >    that point at objects, refs, config, and anything _we_ care
> >    about, as opposed to random garbage (from _our_ point of view)
> >    files third-parties throw into .git/ directory.  Would it have
> >    made a better trade-off if we tried to make the check more
> >    precise, only complaining about the things we care about (in
> >    other words, what _we_ use)
>
> I wondered about that possibility too. But it's not at all clear to
> me how a symlink to .git/objects/foo risks any more security problem
> to git than one to .git/annex/whatever, or indeed to /home/linus/.bashrc.

It risks more security problems because `.git/objects/??/*` is not re-hashed when it is being used by Git. That's a very easy way to slip in unwanted file contents.

And there is a good reason _not_ to write stuff inside the `.git/` directory unless you happen to be, well, Git itself: Git makes no guarantees whatsoever that you can write into that directory whatever you want. A future Git version might even write a file `.git/annex`, breaking `git-annex`' assumptions, and that'd be totally within the guarantees Git makes.

Show 8 quoted lines

> Git clearly has to get the security right of handling working tree files
> that are symlinks.
>
> The security hole that triggered this defense in depth, CVE-2024-32021,
> involved an attacker with write access to .git/objects/ making a symlink
> in there while another repo was cloning it. So it involved symlinks
> inside a remote .git/objects/, which is very different than symlinks
> into .git/objects/.

No, the vulnerability that triggered this defense-in-depth was not CVE-2024-32021, but instead CVE-2024-32002, a critical security issue.

Removing the defense-in-depth makes it more likely for otherwise relatively harmless bugs à la "oh whoops, we wrote something we shouldn't have" to escalate to full, critical Remote Code Execution vulnerabilities.

Ciao, Johannes