85 lines
3.6 KiB
Diff
85 lines
3.6 KiB
Diff
From 150e6b0aedf57d224c3c49038c306477fa159886 Mon Sep 17 00:00:00 2001
|
|
From: Patrick Steinhardt <ps@pks.im>
|
|
Date: Mon, 15 Apr 2024 13:30:26 +0200
|
|
Subject: [PATCH] builtin/clone: stop resolving symlinks when copying files
|
|
|
|
When a user performs a local clone without `--no-local`, then we end up
|
|
copying the source repository into the target repository directly. To
|
|
optimize this even further, we try to hardlink files into place instead
|
|
of copying data over, which helps both disk usage and speed.
|
|
|
|
There is an important edge case in this context though, namely when we
|
|
try to hardlink symlinks from the source repository into the target
|
|
repository. Depending on both platform and filesystem the resulting
|
|
behaviour here can be different:
|
|
|
|
- On macOS and NetBSD, calling link(3P) with a symlink target creates
|
|
a hardlink to the file pointed to by the symlink.
|
|
|
|
- On Linux, calling link(3P) instead creates a hardlink to the symlink
|
|
itself.
|
|
|
|
To unify this behaviour, 36596fd2df (clone: better handle symlinked
|
|
files at .git/objects/, 2019-07-10) introduced logic to resolve symlinks
|
|
before we try to link(3P) files. Consequently, the new behaviour was to
|
|
always create a hard link to the target of the symlink on all platforms.
|
|
|
|
Eventually though, we figured out that following symlinks like this can
|
|
cause havoc when performing a local clone of a malicious repository,
|
|
which resulted in CVE-2022-39253. This issue was fixed via 6f054f9fb3
|
|
(builtin/clone.c: disallow `--local` clones with symlinks, 2022-07-28),
|
|
by refusing symlinks in the source repository.
|
|
|
|
But even though we now shouldn't ever link symlinks anymore, the code
|
|
that resolves symlinks still exists. In the best case the code does not
|
|
end up doing anything because there are no symlinks anymore. In the
|
|
worst case though this can be abused by an adversary that rewrites the
|
|
source file after it has been checked not to be a symlink such that it
|
|
actually is a symlink when we call link(3P). Thus, it is still possible
|
|
to recreate CVE-2022-39253 due to this time-of-check-time-of-use bug.
|
|
|
|
Remove the call to `realpath()`. This doesn't yet address the actual
|
|
vulnerability, which will be handled in a subsequent commit.
|
|
|
|
Reported-by: Apple Product Security <product-security@apple.com>
|
|
Signed-off-by: Patrick Steinhardt <ps@pks.im>
|
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
|
---
|
|
builtin/clone.c | 6 +-----
|
|
1 file changed, 1 insertion(+), 5 deletions(-)
|
|
|
|
diff --git a/builtin/clone.c b/builtin/clone.c
|
|
index 3c2ae31a5..073e6323d 100644
|
|
--- a/builtin/clone.c
|
|
+++ b/builtin/clone.c
|
|
@@ -320,7 +320,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
|
int src_len, dest_len;
|
|
struct dir_iterator *iter;
|
|
int iter_status;
|
|
- struct strbuf realpath = STRBUF_INIT;
|
|
|
|
mkdir_if_missing(dest->buf, 0777);
|
|
|
|
@@ -358,8 +357,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
|
if (unlink(dest->buf) && errno != ENOENT)
|
|
die_errno(_("failed to unlink '%s'"), dest->buf);
|
|
if (!option_no_hardlinks) {
|
|
- strbuf_realpath(&realpath, src->buf, 1);
|
|
- if (!link(realpath.buf, dest->buf))
|
|
+ if (!link(src->buf, dest->buf))
|
|
continue;
|
|
if (option_local > 0)
|
|
die_errno(_("failed to create link '%s'"), dest->buf);
|
|
@@ -373,8 +371,6 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
|
|
strbuf_setlen(src, src_len);
|
|
die(_("failed to iterate over '%s'"), src->buf);
|
|
}
|
|
-
|
|
- strbuf_release(&realpath);
|
|
}
|
|
|
|
static void clone_local(const char *src_repo, const char *dest_repo)
|
|
--
|
|
2.33.0
|
|
|