brian m. carlson wrote:
> If these protections hadn't broken things, I'd agree that we should keep
> them. However, they have broken things and they've introduced a
> serious regression breaking a major project, and we should revert them.
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.
--
see shy jo
[1] https://forum.gitlab.com/t/recent-git-v2-45-1-breaks-git-annex-compatibility-because-of-apparent-fsck-symlinkpointstogitdir-error-on-gitlab/104909
