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

> 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.

> 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

> (+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)