Hi Joey, On Thu, 23 May 2024, Joey Hess wrote: > 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. > 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