Disallow dubiously-nested submodule git directories
Currently it is technically possible to let a submodule's git directory point right into the git dir of a sibling submodule. Example: the git directories of two submodules with the names `hippo` and `hippo/hooks` would be `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively, but the latter is already intended to house the former's hooks. In most cases, this is just confusing, but there is also a (quite contrived) attack vector where Git can be fooled into mistaking remote content for file contents it wrote itself during a recursive clone. Let's plug this bug. To do so, we introduce the new function `validate_submodule_git_dir()` which simply verifies that no git dir exists for any leading directories of the submodule name (if there are any). Note: this patch specifically continues to allow sibling modules names of the form `core/lib`, `core/doc`, etc, as long as `core` is not a submodule name. This fixes CVE-2019-1387. Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Johannes Schindelin committed
Oct 1, 2019 at 23:27 UTC
a8dee3ca610f5a1d403634492136c887f83b59d2
4 files changed
+79
-2
builtin/submodule--helper.c
+4
@@ -678,6 +678,10 @@ static int module_clone(int argc, const char **argv, const char *prefix)
678
} else
679
path = xstrdup(path);
680
681
+ if (validate_submodule_git_dir(sm_gitdir, name) < 0)
682
+ die(_("refusing to create/use '%s' in another submodule's "
683
+ "git dir"), sm_gitdir);
684
+
685
if (!file_exists(sm_gitdir)) {
686
if (safe_create_leading_directories_const(sm_gitdir) < 0)
687
die(_("could not create directory '%s'"), sm_gitdir);
submodule.c
+47
-2
@@ -1842,6 +1842,47 @@ int parallel_submodules(void)
1842
return parallel_jobs;
1843
}
1844
1845
+int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
1846
+{
1847
+ size_t len = strlen(git_dir), suffix_len = strlen(submodule_name);
1848
+ char *p;
1849
+ int ret = 0;
1850
+
1851
+ if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
1852
+ strcmp(p, submodule_name))
1853
+ BUG("submodule name '%s' not a suffix of git dir '%s'",
1854
+ submodule_name, git_dir);
1855
+
1856
+ /*
1857
+ * We prevent the contents of sibling submodules' git directories to
1858
+ * clash.
1859
+ *
1860
+ * Example: having a submodule named `hippo` and another one named
1861
+ * `hippo/hooks` would result in the git directories
1862
+ * `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively,
1863
+ * but the latter directory is already designated to contain the hooks
1864
+ * of the former.
1865
+ */
1866
+ for (; *p; p++) {
1867
+ if (is_dir_sep(*p)) {
1868
+ char c = *p;
1869
+
1870
+ *p = '\0';
1871
+ if (is_git_directory(git_dir))
1872
+ ret = -1;
1873
+ *p = c;
1874
+
1875
+ if (ret < 0)
1876
+ return error(_("submodule git dir '%s' is "
1877
+ "inside git dir '%.*s'"),
1878
+ git_dir,
1879
+ (int)(p - git_dir), git_dir);
1880
+ }
1881
+ }
1882
+
1883
+ return 0;
1884
+}
1885
+
1886
/*
1887
* Embeds a single submodules git directory into the superprojects git dir,
1888
* non recursively.
@@ -1850,7 +1891,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
1891
const char *path)
1892
{
1893
char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
1853
- const char *new_git_dir;
1894
+ char *new_git_dir;
1895
const struct submodule *sub;
1896
1897
if (submodule_uses_worktrees(path))
@@ -1868,10 +1909,14 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
1909
if (!sub)
1910
die(_("could not lookup name for submodule '%s'"), path);
1911
1871
- new_git_dir = git_path("modules/%s", sub->name);
1912
+ new_git_dir = git_pathdup("modules/%s", sub->name);
1913
+ if (validate_submodule_git_dir(new_git_dir, sub->name) < 0)
1914
+ die(_("refusing to move '%s' into an existing git dir"),
1915
+ real_old_git_dir);
1916
if (safe_create_leading_directories_const(new_git_dir) < 0)
1917
die(_("could not create directory '%s'"), new_git_dir);
1918
real_new_git_dir = real_pathdup(new_git_dir, 1);
1919
+ free(new_git_dir);
1920
1921
fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"),
1922
get_super_prefix_or_empty(), path,
submodule.h
+5
@@ -120,6 +120,11 @@ extern int parallel_submodules(void);
120
*/
121
int submodule_to_gitdir(struct strbuf *buf, const char *submodule);
122
123
+/*
124
+ * Make sure that no submodule's git dir is nested in a sibling submodule's.
125
+ */
126
+int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
127
+
128
#define SUBMODULE_MOVE_HEAD_DRY_RUN (1<<0)
129
#define SUBMODULE_MOVE_HEAD_FORCE (1<<1)
130
extern int submodule_move_head(const char *path,
t/t7415-submodule-names.sh
+23
@@ -106,4 +106,27 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '
106
! grep gitdir squatting-clone/d/a/git~2
107
'
108
109
+test_expect_success 'git dirs of sibling submodules must not be nested' '
110
+ git init nested &&
111
+ test_commit -C nested nested &&
112
+ (
113
+ cd nested &&
114
+ cat >.gitmodules <<-EOF &&
115
+ [submodule "hippo"]
116
+ url = .
117
+ path = thing1
118
+ [submodule "hippo/hooks"]
119
+ url = .
120
+ path = thing2
121
+ EOF
122
+ git clone . thing1 &&
123
+ git clone . thing2 &&
124
+ git add .gitmodules thing1 thing2 &&
125
+ test_tick &&
126
+ git commit -m nested
127
+ ) &&
128
+ test_must_fail git clone --recurse-submodules nested clone 2>err &&
129
+ test_i18ngrep "is inside git dir" err
130
+'
131
+
132
test_done