Re: Limited local file inclusion with .mailmap symlinks and git-archive

From: Jeff King <peff@peff.net>
Date: Feb 15, 2021, 23:17 UTC
Message-ID: <YCsBA002yv8XpppM@coredump.intra.peff.net>
In-Reply-To: <CAP3OtXj15f9XV=Rzz2oBXQ1TQH3WWKJGBbeaWrmp6Ha4ZTn9nA@mail.gmail.com>

On Sat, Feb 13, 2021 at 11:49:32AM -0600, Blake Burkhart wrote:

Show 7 quoted lines

> I reported this issue to the private security list first and discussed
> this issue with Peff. This is similar to existing concerns with
> .gitmodules, .gitattributes and .gitignore. Git already disallows
> checking out a .gitmodules file from a repository, and I understand
> there are in progress patches to add similar protection for
> .gitattributes and .gitignore. Please ensure the .mailmap file gets
> similar symlink protection.

Thanks again for bringing this up.

Here are some patches that I think will help. They're meant to be applied on the stalled jk/symlinked-dotgitx-files topic, which Junio has been carrying in "seen" for a few months now.

The sticking point there was that we were concerned that the fsck checks for .gitattributes/.gitignore would catch historical commits in real projects, making them annoying to work with. So the first patch here loosens those checks to warnings. I think this is safe enough, as the real protection is in preventing checkouts in the index code paths (the fsck checks are really just about protecting other clients using older versions, but the severity of these attacks is so low that the tradeoff doesn't make as much sense).

Obviously this could be squashed into the earlier patches, but I think documenting the change of direction with a separate commit makes sense.

And then the second patch adds similar .mailmap support (also as a warning, since I think it is largely in the same boat, and it makes sense to be consistent).

  [1/2]: fsck: make symlinked .gitignore and .gitattributes a warning
  [2/2]: disallow symlinked .mailmap files

 cache.h                      |  1 +
 fsck.c                       | 10 ++++++++--
 path.c                       |  5 +++++
 read-cache.c                 |  6 ++++--
 t/helper/test-path-utils.c   |  5 +++++
 t/t0060-path-utils.sh        | 10 ++++++++++
 t/t7450-bad-dotgitx-files.sh | 26 +++++++++++++++++++-------
 utf8.c                       |  5 +++++
 utf8.h                       |  1 +
 9 files changed, 58 insertions(+), 11 deletions(-)

-Peff