builtin/clone.c: disallow `--local` clones with symlinks

When cloning a repository with `--local`, Git relies on either making a hardlink or copy to every file in the "objects" directory of the source repository. This is done through the callpath `cmd_clone()` -> `clone_local()` -> `copy_or_link_directory()`. The way this optimization works is by enumerating every file and directory recursively in the source repository's `$GIT_DIR/objects` directory, and then either making a copy or hardlink of each file. The only exception to this rule is when copying the "alternates" file, in which case paths are rewritten to be absolute before writing a new "alternates" file in the destination repo. One quirk of this implementation is that it dereferences symlinks when cloning. This behavior was most recently modified in 36596fd2df (clone: better handle symlinked files at .git/objects/, 2019-07-10), which attempted to support `--local` clones of repositories with symlinks in their objects directory in a platform-independent way. Unfortunately, this behavior of dereferencing symlinks (that is, creating a hardlink or copy of the source's link target in the destination repository) can be used as a component in attacking a victim by inadvertently exposing the contents of file stored outside of the repository. Take, for example, a repository that stores a Dockerfile and is used to build Docker images. When building an image, Docker copies the directory contents into the VM, and then instructs the VM to execute the Dockerfile at the root of the copied directory. This protects against directory traversal attacks by copying symbolic links as-is without dereferencing them. That is, if a user has a symlink pointing at their private key material (where the symlink is present in the same directory as the Dockerfile, but the key itself is present outside of that directory), the key is unreadable to a Docker image, since the link will appear broken from the container's point of view. This behavior enables an attack whereby a victim is convinced to clone a repository containing an embedded submodule (with a URL like "file:///proc/self/cwd/path/to/submodule") which has a symlink pointing at a path containing sensitive information on the victim's machine. If a user is tricked into doing this, the contents at the destination of those symbolic links are exposed to the Docker image at runtime. One approach to preventing this behavior is to recreate symlinks in the destination repository. But this is problematic, since symlinking the objects directory are not well-supported. (One potential problem is that when sharing, e.g. a "pack" directory via symlinks, different writers performing garbage collection may consider different sets of objects to be reachable, enabling a situation whereby garbage collecting one repository may remove reachable objects in another repository). Instead, prohibit the local clone optimization when any symlinks are present in the `$GIT_DIR/objects` directory of the source repository. Users may clone the repository again by prepending the "file://" scheme to their clone URL, or by adding the `--no-local` option to their `git clone` invocation. The directory iterator used by `copy_or_link_directory()` must no longer dereference symlinks (i.e., it *must* call `lstat()` instead of `stat()` in order to discover whether or not there are symlinks present). This has no bearing on the overall behavior, since we will immediately `die()` on encounter a symlink. Note that t5604.33 suggests that we do support local clones with symbolic links in the source repository's objects directory, but this was likely unintentional, or at least did not take into consideration the problem with sharing parts of the objects directory with symbolic links at the time. Update this test to reflect which options are and aren't supported. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Taylor Blau committed Jul 28, 2022 at 17:35 UTC 6f054f9fb3a501c35b55c65e547a244f14c38d56
2 files changed +23 -35
builtin/clone.c
+5 -3
@@ -420,13 +420,11 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
420 int src_len, dest_len;
421 struct dir_iterator *iter;
422 int iter_status;
423 - unsigned int flags;
423 struct strbuf realpath = STRBUF_INIT;
424
425 mkdir_if_missing(dest->buf, 0777);
426
428 - flags = DIR_ITERATOR_PEDANTIC | DIR_ITERATOR_FOLLOW_SYMLINKS;
429 - iter = dir_iterator_begin(src->buf, flags);
427 + iter = dir_iterator_begin(src->buf, DIR_ITERATOR_PEDANTIC);
428
429 if (!iter)
430 die_errno(_("failed to start iterator over '%s'"), src->buf);
@@ -442,6 +440,10 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
440 strbuf_setlen(dest, dest_len);
441 strbuf_addstr(dest, iter->relative_path);
442
443 + if (S_ISLNK(iter->st.st_mode))
444 + die(_("symlink '%s' exists, refusing to clone with --local"),
445 + iter->relative_path);
446 +
447 if (S_ISDIR(iter->st.st_mode)) {
448 mkdir_if_missing(dest->buf, 0777);
449 continue;
t/t5604-clone-reference.sh
+18 -32
@@ -300,8 +300,6 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file
300 ln -s ../an-object $obj &&
301
302 cd ../ &&
303 - find . -type f | sort >../../../T.objects-files.raw &&
304 - find . -type l | sort >../../../T.objects-symlinks.raw &&
303 echo unknown_content >unknown_file
304 ) &&
305 git -C T fsck &&
@@ -310,19 +308,27 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file
308
309
310 test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at objects/' '
313 - for option in --local --no-hardlinks --shared --dissociate
311 + # None of these options work when cloning locally, since T has
312 + # symlinks in its `$GIT_DIR/objects` directory
313 + for option in --local --no-hardlinks --dissociate
314 do
315 - git clone $option T T$option || return 1 &&
316 - git -C T$option fsck || return 1 &&
317 - git -C T$option rev-list --all --objects >T$option.objects &&
318 - test_cmp T.objects T$option.objects &&
319 - (
320 - cd T$option/.git/objects &&
321 - find . -type f | sort >../../../T$option.objects-files.raw &&
322 - find . -type l | sort >../../../T$option.objects-symlinks.raw
323 - )
315 + test_must_fail git clone $option T T$option 2>err || return 1 &&
316 + test_i18ngrep "symlink.*exists" err || return 1
317 done &&
318
319 + # But `--shared` clones should still work, even when specifying
320 + # a local path *and* that repository has symlinks present in its
321 + # `$GIT_DIR/objects` directory.
322 + git clone --shared T T--shared &&
323 + git -C T--shared fsck &&
324 + git -C T--shared rev-list --all --objects >T--shared.objects &&
325 + test_cmp T.objects T--shared.objects &&
326 + (
327 + cd T--shared/.git/objects &&
328 + find . -type f | sort >../../../T--shared.objects-files.raw &&
329 + find . -type l | sort >../../../T--shared.objects-symlinks.raw
330 + ) &&
331 +
332 for raw in $(ls T*.raw)
333 do
334 sed -e "s!/../!/Y/!; s![0-9a-f]\{38,\}!Z!" -e "/commit-graph/d" \
@@ -330,26 +336,6 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje
336 sort $raw.de-sha-1 >$raw.de-sha || return 1
337 done &&
338
333 - cat >expected-files <<-EOF &&
334 - ./Y/Z
335 - ./Y/Z
336 - ./Y/Z
337 - ./a-loose-dir/Z
338 - ./an-object
339 - ./info/packs
340 - ./pack/pack-Z.idx
341 - ./pack/pack-Z.pack
342 - ./packs/pack-Z.idx
343 - ./packs/pack-Z.pack
344 - ./unknown_file
345 - EOF
346 -
347 - for option in --local --no-hardlinks --dissociate
348 - do
349 - test_cmp expected-files T$option.objects-files.raw.de-sha || return 1 &&
350 - test_must_be_empty T$option.objects-symlinks.raw.de-sha || return 1
351 - done &&
352 -
339 echo ./info/alternates >expected-files &&
340 test_cmp expected-files T--shared.objects-files.raw &&
341 test_must_be_empty T--shared.objects-symlinks.raw