]> git.ipfire.org Git - thirdparty/git.git/commitdiff
clone --recurse-submodules: prevent name squatting on Windows
authorJohannes Schindelin <johannes.schindelin@gmx.de>
Thu, 12 Sep 2019 12:20:39 +0000 (14:20 +0200)
committerJohannes Schindelin <johannes.schindelin@gmx.de>
Wed, 4 Dec 2019 12:20:05 +0000 (13:20 +0100)
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>
builtin/clone.c
builtin/submodule--helper.c
git-submodule.sh
t/t7415-submodule-names.sh

index f7e17d22951cfd8e498143c009fa0303d0ff8319..44b716923fc6eb2b99b7905d87d21e6cfa22913a 100644 (file)
@@ -757,7 +757,7 @@ static int checkout(int submodule_progress)
 
        if (!err && (option_recurse_submodules.nr > 0)) {
                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");
index 676cfed770f3cce91b4f5eebcb88d17b7f2056b2..79156fac45dd2050e36376d90ae85c0a1ccbb898 100644 (file)
@@ -13,6 +13,7 @@
 #include "remote.h"
 #include "refs.h"
 #include "connect.h"
+#include "dir.h"
 
 static char *get_default_remote(void)
 {
@@ -623,6 +624,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
        char *p, *path = NULL, *sm_gitdir;
        struct strbuf sb = STRBUF_INIT;
        struct string_list reference = STRING_LIST_INIT_NODUP;
+       int require_init = 0;
        char *sm_alternate = NULL, *error_strategy = NULL;
 
        struct option module_clone_options[] = {
@@ -647,6 +649,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()
        };
 
@@ -685,6 +689,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);
@@ -733,6 +739,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;
@@ -748,7 +755,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}
 
@@ -850,6 +857,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", sub->url, NULL);
@@ -992,6 +1001,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()
        };
 
index 8f260fbd9ca4a38ce78d8a29957cabbe3e26fb4b..e4843a58745bff4832de21cf430edf067075ffc4 100755 (executable)
@@ -34,6 +34,7 @@ reference=
 cached=
 recursive=
 init=
+require_init=
 files=
 remote=
 nofetch=
@@ -528,6 +529,10 @@ cmd_update()
                -i|--init)
                        init=1
                        ;;
+               --require-init)
+                       init=1
+                       require_init=1
+                       ;;
                --remote)
                        remote=1
                        ;;
@@ -606,6 +611,7 @@ cmd_update()
                ${update:+--update "$update"} \
                ${reference:+"$reference"} \
                ${depth:+--depth "$depth"} \
+               ${require_init:+--require-init} \
                ${recommend_shallow:+"$recommend_shallow"} \
                ${jobs:+$jobs} \
                "$@" || echo "#unmatched" $?
index 75fa071c6d042aad43d3255935c1d99324073a31..e1cd0a35451774089c39d3495af64143342b28b0 100755 (executable)
@@ -73,4 +73,35 @@ test_expect_success 'clone evil superproject' '
        ! grep "RUNNING POST CHECKOUT" output
 '
 
+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