* Re: Too-loose checks for applying safe.directory rules? [not found] <YlZJGbcNzSp5yNN1@nand.local> @ 2022-04-13 5:26 ` Jonathan Nieder 2022-04-26 11:08 ` CB Bailey 0 siblings, 1 reply; 2+ messages in thread From: Jonathan Nieder @ 2022-04-13 5:26 UTC (permalink / raw) To: Taylor Blau; +Cc: git, Johannes Schindelin, Junio C Hamano (+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 ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Too-loose checks for applying safe.directory rules? 2022-04-13 5:26 ` Too-loose checks for applying safe.directory rules? Jonathan Nieder @ 2022-04-26 11:08 ` CB Bailey 0 siblings, 0 replies; 2+ messages in thread From: CB Bailey @ 2022-04-26 11:08 UTC (permalink / raw) To: Jonathan Nieder, Taylor Blau, Derrick Stolee, Matheus Valadares Cc: git, Johannes Schindelin, Junio C Hamano On 13/04/2022 06:26, Jonathan Nieder wrote: > (+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) I just wanted to send a belated "thank you for fixing this" as the other aspect of this is that if you had any configuration variable configured to an empty string (in my case 'http.proxy'), then any 'safe.directory' which happended to be parsed before this was ignored. This bug was the cause of me running git in a debugger in production for the first time in a very long time as we tried to work out why 'safe.directory' was not working. CB ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-04-26 11:52 UTC | newest] Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <YlZJGbcNzSp5yNN1@nand.local> 2022-04-13 5:26 ` Too-loose checks for applying safe.directory rules? Jonathan Nieder 2022-04-26 11:08 ` CB Bailey
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).