clone_submodule: avoid using access() on directories

In 0060fd1511 (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 <johannes.schindelin@gmx.de>
This commit is contained in:
Johannes Schindelin 2024-04-12 21:00:44 +02:00
parent 9706576133
commit eafffd9ad4

View file

@ -1742,7 +1742,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,
} else {
char *path;
if (clone_data->require_init && !access(clone_data_path, X_OK) &&
if (clone_data->require_init && !stat(clone_data_path, &st) &&
!is_empty_dir(clone_data_path))
die(_("directory not empty: '%s'"), clone_data_path);
if (safe_create_leading_directories_const(clone_data_path) < 0)