Re: What's cooking in git.git (Oct 2022, #06; Wed, 19)

From: Jeff King <peff@peff.net>
Date: Oct 21, 2022, 04:55 UTC
Message-ID: <Y1ImS1Muvk1MAQeC@coredump.intra.peff.net>
In-Reply-To: <Y1Hy9n7E1/yWKkYv@newk>

On Thu, Oct 20, 2022 at 09:16:38PM -0400, Michael McClimon wrote:

Show 13 quoted lines

> > * mm/git-pm-try-catch-syntax-fix (2022-10-17) 1 commit
> >  - Git.pm: add semicolon after catch statement
> > 
> >  Fix a longstanding syntax error in Git.pm error codepath.
> > 
> >  Will merge to 'next'??
> >  source: <20221016212236.12453-2-michael@mcclimon.org>
> 
> I am not totally sure what these question marks mean, but I'm happy for
> this to go to next, for what it's worth. (There was an outstanding
> question about the general behavior of Git.pm in bare unsafe
> repositories, but I certainly am not planning to solve it in this
> series.) Thanks!

Me too. I had hoped for a test, but because of those outstanding questions about the behavior, I think we are better off skipping it for now, and fixing what is obviously a bug.

There is one devil's advocate thing that is nagging at me, though. Because this whole safe-directory thing is security-relevant, could we make things worse by "fixing" it partially? That is, consider this code:

  git init --bare foo.git
  sudo chown -R nobody foo.git
  cd foo.git
  perl -MGit -e 'Git->repository'

Post 2.35.4, etc, is it a security vulnerability for that final step to open the repository? If so, then right now we are "not vulnerable" in the sense that an unrelated bug causes Git.pm to bail in the final step. And fixing that will mean we become vulnerable.

To be clear, having an unrelated bug is not a good way to prevent security vulnerabilities. But it is the status quo. If the fixed version (after your patch to add the semicolon) is vulnerable, but the released one in the hands of users is not, then fixing it does make things worse in the short term (even though the obviously correct thing in the long run is to fix that bug and also correct the vulnerability the right way).

I wasn't around when the directory-ownership stuff was discussed or worked on. But it might be nice for people who worked on it or thought a lot about it to give an opinion on whether the state after your patch is in fact vulnerable to a security flaw.

-Peff