From 850c3a220e7a0b1bf740fba9ac8f3f2b0486a1af Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 28 Mar 2024 09:44:28 +0100 Subject: entry: report more colliding paths In b878579ae7 (clone: report duplicate entries on case-insensitive filesystems, 2018-08-17) code was added to warn about index entries that resolve to the same file system entity (usually the cause is a case-insensitive filesystem). In Git for Windows, where inodes are not trusted (because of a performance trade-off, inodes are equal to 0 by default), that check does not compare inode numbers but the verbatim path. This logic works well when index entries' paths differ only in case. However, for file/directory conflicts only the file's path was reported, leaving the user puzzled with what that path collides. Let's try ot catch colliding paths even if one path is the prefix of the other. We do this also in setups where the file system is case-sensitive because the inode check would not be able to catch those collisions. While not a complete solution (for example, on macOS, Unicode normalization could also lead to file/directory conflicts but be missed by this logic), it is at least another defensive layer on top of what the previous commits added. Signed-off-by: Johannes Schindelin --- entry.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'entry.c') diff --git a/entry.c b/entry.c index 616e4f073c..a4c18c5645 100644 --- a/entry.c +++ b/entry.c @@ -454,7 +454,7 @@ static void mark_colliding_entries(const struct checkout *state, continue; if ((trust_ino && !match_stat_data(&dup->ce_stat_data, st)) || - (!trust_ino && !fspathcmp(ce->name, dup->name))) { + paths_collide(ce->name, dup->name)) { dup->ce_flags |= CE_MATCHED; break; } -- cgit v1.2.3 From 31572dc420afee36db8fbbbe060dd78c9a48778c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 28 Mar 2024 10:55:07 +0100 Subject: clone: when symbolic links collide with directories, keep the latter When recursively cloning a repository with submodules, we must ensure that the submodules paths do not suddenly contain symbolic links that would let Git write into unintended locations. We just plugged that vulnerability, but let's add some more defense-in-depth. Since we can only keep one item on disk if multiple index entries' paths collide, we may just as well avoid keeping a symbolic link (because that would allow attack vectors where Git follows those links by mistake). Technically, we handle more situations than cloning submodules into paths that were (partially) replaced by symbolic links. This provides defense-in-depth in case someone finds a case-folding confusion vulnerability in the future that does not even involve submodules. Signed-off-by: Johannes Schindelin --- entry.c | 14 ++++++++++++++ t/t5601-clone.sh | 15 +++++++++++++++ t/t7406-submodule-update.sh | 4 ++-- 3 files changed, 31 insertions(+), 2 deletions(-) (limited to 'entry.c') diff --git a/entry.c b/entry.c index a4c18c5645..1d78e54168 100644 --- a/entry.c +++ b/entry.c @@ -541,6 +541,20 @@ int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca, /* If it is a gitlink, leave it alone! */ if (S_ISGITLINK(ce->ce_mode)) return 0; + /* + * We must avoid replacing submodules' leading + * directories with symbolic links, lest recursive + * clones can write into arbitrary locations. + * + * Technically, this logic is not limited + * to recursive clones, or for that matter to + * submodules' paths colliding with symbolic links' + * paths. Yet it strikes a balance in favor of + * simplicity, and if paths are colliding, we might + * just as well keep the directories during a clone. + */ + if (state->clone && S_ISLNK(ce->ce_mode)) + return 0; remove_subtree(&path); } else if (unlink(path.buf)) return error_errno("unable to unlink old '%s'", path.buf); diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index b2524a24c2..fd02984330 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -633,6 +633,21 @@ test_expect_success CASE_INSENSITIVE_FS 'colliding file detection' ' test_i18ngrep "the following paths have collided" icasefs/warning ' +test_expect_success CASE_INSENSITIVE_FS,SYMLINKS \ + 'colliding symlink/directory keeps directory' ' + git init icasefs-colliding-symlink && + ( + cd icasefs-colliding-symlink && + a=$(printf a | git hash-object -w --stdin) && + printf "100644 %s 0\tA/dir/b\n120000 %s 0\ta\n" $a $a >idx && + git update-index --index-info err && - grep "directory not empty" err && + git clone --recursive captain hooked 2>err && + ! grep HOOK-RUN err && test_path_is_missing "$tell_tale_path" ' -- cgit v1.2.3