Re: [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS

brian m. carlson wrote:

More than one major project; they also broke git-annex in the case where a git-annex repository, which contains symlinks into .git/annex/objects/, is pushed to a bare repository with receive.fsckObjects set. (Gitlab is currently affected[1].)

BTW, do I understand correctly that the defence in depth patch set was developed under embargo and has never been publically reviewed?

Looking at commit a33fea0886cfa016d313d2bd66bdd08615bffbc9, I noticed that its PATH_MAX check is also dodgy due to that having values ranging from 260 (Windows) to 1024 (Freebsd) to 4096 (Linux), which means git repositories containing legitimate, working symlinks can now fail to be pushed depending on what OS happens to host a reciving bare repository.

+                               if (is_ntfs_dotgit(p))

This means that symlinks to eg "git~1" are also warned about, which seems strange behavior on eg Linux.

+                               backslash = memchr(p, '\\', slash - p);

This and other backslash handling code for some reason is also run on linux, so a symlink to eg "ummmm\\git~1" is also warned about.

+               if (!buf || size > PATH_MAX) {

I suspect, but have not confirmed, that this is allows a symlink target 1 byte longer than the OS supports, because PATH_MAX includes a trailing NUL.

All in all, this seems to need more review and a more careful consideration of breakage now that the security holes are not under embargo.