From: Joey Hess <id@joeyh.name>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
Johannes Schindelin <Johannes.Schindelin@gmx.de>,
git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS
Date: Wed, 22 May 2024 05:49:12 -0400 [thread overview]
Message-ID: <Zk2_mJpE7tJgqxSp@kitenet.net> (raw)
In-Reply-To: <ZkO-b6Nswrn9H7Ed@tapette.crustytoothpaste.net>
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
next prev parent reply other threads:[~2024-05-22 10:32 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 18:16 [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS brian m. carlson
2024-05-14 18:16 ` [PATCH 1/2] Revert "clone: prevent hooks from running during a clone" brian m. carlson
2024-05-14 18:16 ` [PATCH 2/2] Revert "core.hooksPath: add some protection while cloning" brian m. carlson
2024-05-14 19:07 ` [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS Johannes Schindelin
2024-05-14 19:41 ` brian m. carlson
2024-05-22 9:49 ` Joey Hess [this message]
2024-05-27 19:35 ` Johannes Schindelin
2024-05-28 2:13 ` Joey Hess
[not found] ` <ZlZSZ1-0F2DEp9yV@tapette.crustytoothpaste.net>
2024-05-28 23:46 ` Junio C Hamano
2024-05-29 8:54 ` Jeff King
2024-05-29 12:17 ` Johannes Schindelin
2024-05-29 16:17 ` Junio C Hamano
2024-05-30 8:17 ` Jeff King
2024-05-24 17:37 ` Joey Hess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zk2_mJpE7tJgqxSp@kitenet.net \
--to=id@joeyh.name \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).