| From f4e9ab82ab3230776d92b9dd7a007fe66929d21a Mon Sep 17 00:00:00 2001 |
| From: Johannes Schindelin <johannes.schindelin@gmx.de> |
| Date: Thu, 12 Sep 2019 14:20:39 +0200 |
| Subject: clone --recurse-submodules: prevent name squatting on Windows |
| |
| commit 0060fd1511b94c918928fa3708f69a3f33895a4a upstream. |
| |
| In addition to preventing `.git` from being tracked by Git, on Windows |
| we also have to prevent `git~1` from being tracked, as the default NTFS |
| short name (also known as the "8.3 filename") for the file name `.git` |
| is `git~1`, otherwise it would be possible for malicious repositories to |
| write directly into the `.git/` directory, e.g. a `post-checkout` hook |
| that would then be executed _during_ a recursive clone. |
| |
| When we implemented appropriate protections in 2b4c6efc821 (read-cache: |
| optionally disallow NTFS .git variants, 2014-12-16), we had analyzed |
| carefully that the `.git` directory or file would be guaranteed to be |
| the first directory entry to be written. Otherwise it would be possible |
| e.g. for a file named `..git` to be assigned the short name `git~1` and |
| subsequently, the short name generated for `.git` would be `git~2`. Or |
| `git~3`. Or even `~9999999` (for a detailed explanation of the lengths |
| we have to go to protect `.gitmodules`, see the commit message of |
| e7cb0b4455c (is_ntfs_dotgit: match other .git files, 2018-05-11)). |
| |
| However, by exploiting two issues (that will be addressed in a related |
| patch series close by), it is currently possible to clone a submodule |
| into a non-empty directory: |
| |
| - On Windows, file names cannot end in a space or a period (for |
| historical reasons: the period separating the base name from the file |
| extension was not actually written to disk, and the base name/file |
| extension was space-padded to the full 8/3 characters, respectively). |
| Helpfully, when creating a directory under the name, say, `sub.`, that |
| trailing period is trimmed automatically and the actual name on disk |
| is `sub`. |
| |
| This means that while Git thinks that the submodule names `sub` and |
| `sub.` are different, they both access `.git/modules/sub/`. |
| |
| - While the backslash character is a valid file name character on Linux, |
| it is not so on Windows. As Git tries to be cross-platform, it |
| therefore allows backslash characters in the file names stored in tree |
| objects. |
| |
| Which means that it is totally possible that a submodule `c` sits next |
| to a file `c\..git`, and on Windows, during recursive clone a file |
| called `..git` will be written into `c/`, of course _before_ the |
| submodule is cloned. |
| |
| Note that the actual exploit is not quite as simple as having a |
| submodule `c` next to a file `c\..git`, as we have to make sure that the |
| directory `.git/modules/b` already exists when the submodule is checked |
| out, otherwise a different code path is taken in `module_clone()` that |
| does _not_ allow a non-empty submodule directory to exist already. |
| |
| Even if we will address both issues nearby (the next commit will |
| disallow backslash characters in tree entries' file names on Windows, |
| and another patch will disallow creating directories/files with trailing |
| spaces or periods), it is a wise idea to defend in depth against this |
| sort of attack vector: when submodules are cloned recursively, we now |
| _require_ the directory to be empty, addressing CVE-2019-1349. |
| |
| Note: the code path we patch is shared with the code path of `git |
| submodule update --init`, which must not expect, in general, that the |
| directory is empty. Hence we have to introduce the new option |
| `--force-init` and hand it all the way down from `git submodule` to the |
| actual `git submodule--helper` process that performs the initial clone. |
| |
| Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> |
| Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> |
| Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> |
| --- |
| builtin/clone.c | 2 +- |
| builtin/submodule--helper.c | 13 ++++++++++++- |
| git-submodule.sh | 6 ++++++ |
| t/t7415-submodule-names.sh | 31 +++++++++++++++++++++++++++++++ |
| 4 files changed, 50 insertions(+), 2 deletions(-) |
| |
| diff --git a/builtin/clone.c b/builtin/clone.c |
| index 6c76a6ed66..2e5543ac50 100644 |
| --- a/builtin/clone.c |
| +++ b/builtin/clone.c |
| @@ -735,7 +735,7 @@ static int checkout(int submodule_progress) |
| |
| if (!err && option_recursive) { |
| struct argv_array args = ARGV_ARRAY_INIT; |
| - argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL); |
| + argv_array_pushl(&args, "submodule", "update", "--require-init", "--recursive", NULL); |
| |
| if (option_shallow_submodules == 1) |
| argv_array_push(&args, "--depth=1"); |
| diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c |
| index b6801b6c7a..9e0f985501 100644 |
| --- a/builtin/submodule--helper.c |
| +++ b/builtin/submodule--helper.c |
| @@ -12,6 +12,7 @@ |
| #include "remote.h" |
| #include "refs.h" |
| #include "connect.h" |
| +#include "dir.h" |
| |
| static char *get_default_remote(void) |
| { |
| @@ -584,6 +585,7 @@ static int module_clone(int argc, const char **argv, const char *prefix) |
| struct strbuf rel_path = STRBUF_INIT; |
| struct strbuf sb = STRBUF_INIT; |
| struct string_list reference = STRING_LIST_INIT_NODUP; |
| + int require_init = 0; |
| |
| struct option module_clone_options[] = { |
| OPT_STRING(0, "prefix", &prefix, |
| @@ -607,6 +609,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) |
| OPT__QUIET(&quiet, "Suppress output for cloning a submodule"), |
| OPT_BOOL(0, "progress", &progress, |
| N_("force cloning progress")), |
| + OPT_BOOL(0, "require-init", &require_init, |
| + N_("disallow cloning into non-empty directory")), |
| OPT_END() |
| }; |
| |
| @@ -645,6 +649,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) |
| die(_("clone of '%s' into submodule path '%s' failed"), |
| url, path); |
| } else { |
| + if (require_init && !access(path, X_OK) && !is_empty_dir(path)) |
| + die(_("directory not empty: '%s'"), path); |
| if (safe_create_leading_directories_const(path) < 0) |
| die(_("could not create directory '%s'"), path); |
| strbuf_addf(&sb, "%s/index", sm_gitdir); |
| @@ -695,6 +701,7 @@ struct submodule_update_clone { |
| int quiet; |
| int recommend_shallow; |
| struct string_list references; |
| + unsigned require_init; |
| const char *depth; |
| const char *recursive_prefix; |
| const char *prefix; |
| @@ -710,7 +717,7 @@ struct submodule_update_clone { |
| int failed_clones_nr, failed_clones_alloc; |
| }; |
| #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \ |
| - SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \ |
| + SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \ |
| NULL, NULL, NULL, \ |
| STRING_LIST_INIT_DUP, 0, NULL, 0, 0} |
| |
| @@ -820,6 +827,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, |
| argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL); |
| if (suc->recommend_shallow && sub->recommend_shallow == 1) |
| argv_array_push(&child->args, "--depth=1"); |
| + if (suc->require_init) |
| + argv_array_push(&child->args, "--require-init"); |
| argv_array_pushl(&child->args, "--path", sub->path, NULL); |
| argv_array_pushl(&child->args, "--name", sub->name, NULL); |
| argv_array_pushl(&child->args, "--url", url, NULL); |
| @@ -963,6 +972,8 @@ static int update_clone(int argc, const char **argv, const char *prefix) |
| OPT__QUIET(&suc.quiet, N_("don't print cloning progress")), |
| OPT_BOOL(0, "progress", &suc.progress, |
| N_("force cloning progress")), |
| + OPT_BOOL(0, "require-init", &suc.require_init, |
| + N_("disallow cloning into non-empty directory")), |
| OPT_END() |
| }; |
| |
| diff --git a/git-submodule.sh b/git-submodule.sh |
| index cee4ddc04b..81495c018f 100755 |
| --- a/git-submodule.sh |
| +++ b/git-submodule.sh |
| @@ -37,6 +37,7 @@ reference= |
| cached= |
| recursive= |
| init= |
| +require_init= |
| files= |
| remote= |
| nofetch= |
| @@ -510,6 +511,10 @@ cmd_update() |
| -i|--init) |
| init=1 |
| ;; |
| + --require-init) |
| + init=1 |
| + require_init=1 |
| + ;; |
| --remote) |
| remote=1 |
| ;; |
| @@ -588,6 +593,7 @@ cmd_update() |
| ${update:+--update "$update"} \ |
| ${reference:+"$reference"} \ |
| ${depth:+--depth "$depth"} \ |
| + ${require_init:+--require-init} \ |
| ${recommend_shallow:+"$recommend_shallow"} \ |
| ${jobs:+$jobs} \ |
| "$@" || echo "#unmatched" $? |
| diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh |
| index 0b2a51ffc8..b3a421d4fd 100755 |
| --- a/t/t7415-submodule-names.sh |
| +++ b/t/t7415-submodule-names.sh |
| @@ -152,4 +152,35 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' |
| ) |
| ' |
| |
| +test_expect_success MINGW 'prevent git~1 squatting on Windows' ' |
| + git init squatting && |
| + ( |
| + cd squatting && |
| + mkdir a && |
| + touch a/..git && |
| + git add a/..git && |
| + test_tick && |
| + git commit -m initial && |
| + |
| + modules="$(test_write_lines \ |
| + "[submodule \"b.\"]" "url = ." "path = c" \ |
| + "[submodule \"b\"]" "url = ." "path = d\\\\a" | |
| + git hash-object -w --stdin)" && |
| + rev="$(git rev-parse --verify HEAD)" && |
| + hash="$(echo x | git hash-object -w --stdin)" && |
| + git update-index --add \ |
| + --cacheinfo 100644,$modules,.gitmodules \ |
| + --cacheinfo 160000,$rev,c \ |
| + --cacheinfo 160000,$rev,d\\a \ |
| + --cacheinfo 100644,$hash,d./a/x \ |
| + --cacheinfo 100644,$hash,d./a/..git && |
| + test_tick && |
| + git commit -m "module" |
| + ) && |
| + test_must_fail git \ |
| + clone --recurse-submodules squatting squatting-clone 2>err && |
| + test_i18ngrep "directory not empty" err && |
| + ! grep gitdir squatting-clone/d/a/git~2 |
| +' |
| + |
| test_done |
| -- |
| 2.24.0.393.g34dc348eaf |
| |