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

[Jeff King <peff@peff.net>]

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

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