Re: Too-loose checks for applying safe.directory rules?
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Apr 13, 2022, 05:26 UTC
- Message-ID
- <YlZe766xu9mHWNdy@google.com>
- In-Reply-To
- <YlZJGbcNzSp5yNN1@nand.local>
(+cc: git@vger, git-security -> bcc) Hi,
Taylor Blau wrote:
Show 41 quoted lines
> 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.
Show 12 quoted lines
> 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