Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

On Tue, Nov 08, 2016 at 08:38:55AM +0700, Duy Nguyen wrote:

quoted

Another approach is to have a config option to disallow symlinks to
destinations outside of the repository tree (I'm not sure if it should
be on or off by default, though).

Let's err on the safe side and disable symlinks to outside repo by
default (or even all symlinks on .gitattributes and .gitignore as the
first step)

Both of those are actually much harder than you might think.

For matching specific names, we have to deal with case-folding.  It's
easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
this is actually protection against malicious repositories, we have to
match all of the horrible filesystem-specific junk that we did for
".git".

Symlinks are likewise tricky.  If we see that a symlink points to
"foo/../bar", then we don't know if it leaves the repository unless we
also look at "foo" to see if it is also a symlink. So you really end up
having to resolve the symlink yourself (and when checking out multiple
files, there's an ordering dependency).

I think it might be enough to check:

  - leading "../" tokens in the symlink's destination can be checked
    against the symlink's path. So "../foo" is OK for path "one/two",
    but not for path "one".

  - interior "../" can be disallowed entirely. Technically
    "foo/../bar/../baz" _can_ be a fine symlink destination, but why?
    It's identical to "baz" unless you are following a bunch of interior
    symlinks. And if those are interior symlinks, it's still confusing
    and unnecessarily obfuscated, and a good sign that somebody is
    trying to do something tricky.

So one reasonable fix might be to have a config option like
"core.saneSymlinks" that enforces both of those rules for _all_ symlinks
that we checkout to the working tree. And it could either refuse to
check them out, or replace them with a file containing the symlink
content (as we do on systems that don't support symlinks, IIRC).

It could even be off by default (for backwards compatibility, as there
really are uses for symlinks reaching out of the repository in some
cases), but people cloning untrusted repos could flip it on. That seems
like an improvement over the current state.

What I learned from my changes in .gitignore is, if we have not
forbidden something, people likely find some creative use for it. As
long as it's can be turned on or off, i guess those minority will stay
happy.

Yes, it's one of the fun things about working on a 10-year-old project.
:)

-Peff