Skip to content

Commit eafffd9

Browse files
committed
clone_submodule: avoid using access() on directories
In 0060fd1 (clone --recurse-submodules: prevent name squatting on Windows, 2019-09-12), I introduced code to verify that a git dir either does not exist, or is at least empty, to fend off attacks where an inadvertently (and likely maliciously) pre-populated git dir would be used while cloning submodules recursively. The logic used `access(<path>, X_OK)` to verify that a directory exists before calling `is_empty_dir()` on it. That is a curious way to check for a directory's existence and might well fail for unwanted reasons. Even the original author (it was I ;-) ) struggles to explain why this function was used rather than `stat()`. This code was _almost_ copypastad in the previous commit, but that `access()` call was caught during review. Let's use `stat()` instead also in the code that was almost copied verbatim. Let's not use `lstat()` because in the unlikely event that somebody snuck a symbolic link in, pointing to a crafted directory, we want to verify that that directory is empty. Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 9706576 commit eafffd9

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

builtin/submodule--helper.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1742,7 +1742,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,
17421742
} else {
17431743
char *path;
17441744

1745-
if (clone_data->require_init && !access(clone_data_path, X_OK) &&
1745+
if (clone_data->require_init && !stat(clone_data_path, &st) &&
17461746
!is_empty_dir(clone_data_path))
17471747
die(_("directory not empty: '%s'"), clone_data_path);
17481748
if (safe_create_leading_directories_const(clone_data_path) < 0)

0 commit comments

Comments
 (0)