Re: Too-loose checks for applying safe.directory rules?

[Jonathan Nieder <jrnieder@gmail.com>]

(+cc: git@vger, git-security -> bcc)
Hi,

Taylor Blau wrote:

> Hi all,
>
> I was skimming the Hacker News comments on my blog post covering the
> latest pair of CVEs, and this[1] comment stuck out to me.
>
> Looking at 8959555cee (setup_git_directory(): add an owner check for the
> top-level directory, 2022-03-02), I wonder why the `safe_directory_cb()`
> callback doesn't bother to check that `key` is `safe.directory`.
>
> Indeed, our checks seem too loose here. Initializing a repository as
> root:
>
>     $ su
>     # git init repo
>
> Then trying to run "git status" inside of that repo as my normal user
> gives the expected error:
>
>     $ git status
>     fatal: unsafe repository ('/home/repo' is owned by someone else)
>     To add an exception for this directory, call:
>
>       git config --global --add safe.directory /home/repo
>
> But doing the following:
>
>     $ git config --global --add foo.bar /home/repo
>
> tricks Git into thinking that _any_ value which looks like a path in the
> "early config" scope can be interpreted as if the key were
> safe.directory, even when it is not:
>
>     $ git status
>     On branch master
>
>     No commits yet
>
>     nothing to commit (create/copy files and use "git add" to track)
>
> The author of [1] sent a PR to the git/git repo on GitHub [2], so I
> don't think there's any value in doing another coordinated release here.

Thanks, Taylor.  I'm taking the liberty of moving to the main Git list.

> We should certainly fix this before 2.36 is released, but should
> probably apply those patches down to the suite of minor versions
> released today, too.
>
> It's entirely possible I'm holding it wrong and/or missing something
> here, and I'd be happy to be wrong here.
>
> Thanks,
> Taylor
>
> [1]: https://news.ycombinator.com/item?id=31010604
> [2]: https://github.com/git/git/pull/1235

Thanks,
Jonathan