From: Johannes Schindelin Date: Fri, 12 Apr 2024 19:00:44 +0000 (+0200) Subject: clone_submodule: avoid using `access()` on directories X-Git-Tag: v2.39.4~3^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=eafffd9ad417bdf0a3c63e5276d5a18f563cd291;p=thirdparty%2Fgit.git clone_submodule: avoid using `access()` on directories In 0060fd1511b (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(, 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 --- diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 4c1a7dbcda..9eacc43574 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -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)