diff options
| author | Johannes Schindelin <johannes.schindelin@gmx.de> | 2024-04-12 21:00:44 +0200 |
|---|---|---|
| committer | Johannes Schindelin <johannes.schindelin@gmx.de> | 2024-04-17 22:30:03 +0200 |
| commit | eafffd9ad417bdf0a3c63e5276d5a18f563cd291 (patch) | |
| tree | 70ff5a94ec5161480c751ad39256fc7f47b56846 /builtin/submodule--helper.c | |
| parent | submodules: submodule paths must not contain symlinks (diff) | |
| download | git-eafffd9ad417bdf0a3c63e5276d5a18f563cd291.tar.xz git-eafffd9ad417bdf0a3c63e5276d5a18f563cd291.zip | |
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(<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>
Diffstat (limited to '')
| -rw-r--r-- | builtin/submodule--helper.c | 2 |
1 files changed, 1 insertions, 1 deletions
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) |