From: Glen Choo <chooglen@xxxxxxxxxx>
6f054f9fb3 (builtin/clone.c: disallow --local clones with
symlinks, 2022-07-28) gives a good error message when "git clone
--local" fails when the repo to clone has symlinks in
"$GIT_DIR/objects". In bffc762f87 (dir-iterator: prevent top-level
symlinks without FOLLOW_SYMLINKS, 2023-01-24), we later extended this
restriction to the case where "$GIT_DIR/objects" is itself a symlink,
but we didn't update the error message then - bffc762f87's tests show
that we print a generic "failed to start iterator over" message.
This is exacerbated by the fact that Documentation/git-clone.txt
mentions neither restriction, so users are left wondering if this is
intentional behavior or not.
Fix this by adding a check to builtin/clone.c: when doing a local clone,
perform an extra check to see if "$GIT_DIR/objects" is a symlink, and if
so, assume that that was the reason for the failure and report the
relevant information. Ideally, dir_iterator_begin() would tell us that
the real failure reason is the presence of the symlink, but (as far as I
can tell) there isn't an appropriate errno value for that.
Also, update Documentation/git-clone.txt to reflect that this
restriction exists.
Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
---
clone: error specifically with --local and symlinked objects
We noticed this because repo [1] creates Git repos where
"$GIT_DIR/objects" is a symlink, and users have gotten confused as to
whether this was intended behavior or not.
I'm no good with lstat() and errno, so if there's a better to do this,
I'd appreciate the input :)
[1] https://gerrit.googlesource.com/git-repo
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1488%2Fchooglen%2Fpush-nymnqqnttzxz-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1488/chooglen/push-nymnqqnttzxz-v1
Pull-Request: https://github.com/git/git/pull/1488
Documentation/git-clone.txt | 5 +++++
builtin/clone.c | 7 ++++++-
t/t5604-clone-reference.sh | 2 +-
3 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index d6434d262d6..c37c4a37f74 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -58,6 +58,11 @@ never use the local optimizations). Specifying `--no-local` will
override the default when `/path/to/repo` is given, using the regular
Git transport instead.
+
+If the repository's `$GIT_DIR/objects` has symbolic links or is a
+symbolic link, the clone will fail. This is a security measure to
+prevent the unintentional copying of files by dereferencing the symbolic
+links.
++
*NOTE*: this operation can race with concurrent modification to the
source repository, similar to running `cp -r src dst` while modifying
`src`.
diff --git a/builtin/clone.c b/builtin/clone.c
index 462c286274c..74ec5b8b02a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -327,8 +327,13 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);
- if (!iter)
+ if (!iter) {
+ struct stat st;
+ if (lstat(src->buf, &st) >= 0 && S_ISLNK(st.st_mode))
+ die(_("'%s' is a symlink, refusing to clone with --local"),
+ src->buf);
die_errno(_("failed to start iterator over '%s'"), src->buf);
+ }
strbuf_addch(src, '/');
src_len = src->len;
diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh
index 83e3c97861d..9845fc04d59 100755
--- a/t/t5604-clone-reference.sh
+++ b/t/t5604-clone-reference.sh
@@ -358,7 +358,7 @@ test_expect_success SYMLINKS 'clone repo with symlinked objects directory' '
test_must_fail git clone --local malicious clone 2>err &&
test_path_is_missing clone &&
- grep "failed to start iterator over" err
+ grep "is a symlink, refusing to clone with --local" err
'
test_done
base-commit: 27d43aaaf50ef0ae014b88bba294f93658016a2e
--
gitgitgadget
