debian/patches/clone-recurse-submodules-prevent-name-squatting-on-Wi.diff - jrn/git - Git at Google

 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
	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