Merge branch 'win32-filenames-cannot-have-trailing-spaces-or-periods'

On Windows, filenames cannot have trailing spaces or periods, when
opening such paths, they are stripped automatically. Read: you can open
the file `README` via the file name `README . . .`. This ambiguity can
be used in combination with other security bugs to cause e.g. remote
code execution during recursive clones. This patch series fixes that.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

compat/mingw.c

@@ -333,6 +333,12 @@ int mingw_mkdir(const char *path, int mode)
 {
 	int ret;
 	wchar_t wpath[MAX_PATH];
+
+	if (!is_valid_win32_path(path)) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	if (xutftowcs_path(wpath, path) < 0)
 		return -1;
 	ret = _wmkdir(wpath);
@@ -345,13 +351,18 @@ int mingw_open (const char *filename, int oflags, ...)
 {
 	va_list args;
 	unsigned mode;
-	int fd;
+	int fd, create = (oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL);
 	wchar_t wfilename[MAX_PATH];
 
 	va_start(args, oflags);
 	mode = va_arg(args, int);
 	va_end(args);
 
+	if (!is_valid_win32_path(filename)) {
+		errno = create ? EINVAL : ENOENT;
+		return -1;
+	}
+
 	if (filename && !strcmp(filename, "/dev/null"))
 		filename = "nul";
 
@@ -413,6 +424,11 @@ FILE *mingw_fopen (const char *filename, const char *otype)
 	int hide = needs_hiding(filename);
 	FILE *file;
 	wchar_t wfilename[MAX_PATH], wotype[4];
+	if (!is_valid_win32_path(filename)) {
+		int create = otype && strchr(otype, 'w');
+		errno = create ? EINVAL : ENOENT;
+		return NULL;
+	}
 	if (filename && !strcmp(filename, "/dev/null"))
 		filename = "nul";
 	if (xutftowcs_path(wfilename, filename) < 0 ||
@@ -435,6 +451,11 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
 	int hide = needs_hiding(filename);
 	FILE *file;
 	wchar_t wfilename[MAX_PATH], wotype[4];
+	if (!is_valid_win32_path(filename)) {
+		int create = otype && strchr(otype, 'w');
+		errno = create ? EINVAL : ENOENT;
+		return NULL;
+	}
 	if (filename && !strcmp(filename, "/dev/null"))
 		filename = "nul";
 	if (xutftowcs_path(wfilename, filename) < 0 ||
@@ -2109,6 +2130,40 @@ static void setup_windows_environment(void)
 		setenv("TERM", "cygwin", 1);
 }
 
+int is_valid_win32_path(const char *path)
+{
+	int preceding_space_or_period = 0, i = 0, periods = 0;
+
+	if (!protect_ntfs)
+		return 1;
+
+	for (;;) {
+		char c = *(path++);
+		switch (c) {
+		case '\0':
+		case '/': case '\\':
+			/* cannot end in ` ` or `.`, except for `.` and `..` */
+			if (preceding_space_or_period &&
+			    (i != periods || periods > 2))
+				return 0;
+			if (!c)
+				return 1;
+
+			i = periods = preceding_space_or_period = 0;
+			continue;
+		case '.':
+			periods++;
+			/* fallthru */
+		case ' ':
+			preceding_space_or_period = 1;
+			i++;
+			continue;
+		}
+		preceding_space_or_period = 0;
+		i++;
+	}
+}
+
 /*
  * Disable MSVCRT command line wildcard expansion (__getmainargs called from
  * mingw startup code, see init.c in mingw runtime).

compat/mingw.h

@@ -428,6 +428,17 @@ int mingw_offset_1st_component(const char *path);
 #include <inttypes.h>
 #endif
 
+/**
+ * Verifies that the given path is a valid one on Windows.
+ *
+ * In particular, path segments are disallowed which end in a period or a
+ * space (except the special directories `.` and `..`).
+ *
+ * Returns 1 upon success, otherwise 0.
+ */
+int is_valid_win32_path(const char *path);
+#define is_valid_path(path) is_valid_win32_path(path)
+
 /**
  * Converts UTF-8 encoded string to UTF-16LE.
  *

git-compat-util.h

@@ -370,6 +370,10 @@ static inline int git_offset_1st_component(const char *path)
 #define offset_1st_component git_offset_1st_component
 #endif
 
+#ifndef is_valid_path
+#define is_valid_path(path) 1
+#endif
+
 #ifndef find_last_dir_sep
 static inline char *git_find_last_dir_sep(const char *path)
 {

read-cache.c

@@ -847,6 +847,9 @@ int verify_path(const char *path, unsigned mode)
 	if (has_dos_drive_prefix(path))
 		return 0;
 
+	if (!is_valid_path(path))
+		return 0;
+
 	goto inside;
 	for (;;) {
 		if (!c)

t/helper/test-path-utils.c

@@ -386,6 +386,23 @@ int cmd_main(int argc, const char **argv)
 	if (argc > 1 && !strcmp(argv[1], "protect_ntfs_hfs"))
 		return !!protect_ntfs_hfs_benchmark(argc - 1, argv + 1);
 
+	if (argc > 1 && !strcmp(argv[1], "is_valid_path")) {
+		int res = 0, expect = 1, i;
+
+		for (i = 2; i < argc; i++)
+			if (!strcmp("--not", argv[i]))
+				expect = 0;
+			else if (expect != is_valid_path(argv[i]))
+				res = error("'%s' is%s a valid path",
+					    argv[i], expect ? " not" : "");
+			else
+				fprintf(stderr,
+					"'%s' is%s a valid path\n",
+					argv[i], expect ? "" : " not");
+
+		return !!res;
+	}
+
 	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
 		argv[1] ? argv[1] : "(there was none)");
 	return 1;

t/t0060-path-utils.sh

@@ -440,4 +440,18 @@ test_expect_success 'match .gitmodules' '
 		.gitmodules,:\$DATA
 '
 
+test_expect_success MINGW 'is_valid_path() on Windows' '
+       test-path-utils is_valid_path \
+		win32 \
+		"win32 x" \
+		../hello.txt \
+		\
+		--not \
+		"win32 "  \
+		"win32 /x "  \
+		"win32."  \
+		"win32 . ." \
+		.../hello.txt
+'
+
 test_done

t/t6130-pathspec-noglob.sh

@@ -10,6 +10,7 @@ test_expect_success 'create commits with glob characters' '
 	# the name "f*" in the worktree, because it is not allowed
 	# on Windows (the tests below do not depend on the presence
 	# of the file in the worktree)
+	git config core.protectNTFS false &&
 	git update-index --add --cacheinfo 100644 "$(git rev-parse HEAD:foo)" "f*" &&
 	test_tick &&
 	git commit -m star &&

t/t7415-submodule-names.sh

@@ -102,7 +102,7 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '
 	) &&
 	test_must_fail git -c core.protectNTFS=false \
 		clone --recurse-submodules squatting squatting-clone 2>err &&
-	test_i18ngrep "directory not empty" err &&
+	test_i18ngrep -e "directory not empty" -e "not an empty directory" err &&
 	! grep gitdir squatting-clone/d/a/git~2
 '
 

t/t7417-submodule-path-url.sh

@@ -17,4 +17,21 @@ test_expect_success 'clone rejects unprotected dash' '
 	test_i18ngrep ignoring err
 '
 
+test_expect_success MINGW 'submodule paths disallows trailing spaces' '
+	git init super &&
+	test_must_fail git -C super submodule add ../upstream "sub " &&
+
+	: add "sub", then rename "sub" to "sub ", the hard way &&
+	git -C super submodule add ../upstream sub &&
+	tree=$(git -C super write-tree) &&
+	git -C super ls-tree $tree >tree &&
+	sed "s/sub/sub /" <tree >tree.new &&
+	tree=$(git -C super mktree <tree.new) &&
+	commit=$(echo with space | git -C super commit-tree $tree) &&
+	git -C super update-ref refs/heads/master $commit &&
+
+	test_must_fail git clone --recurse-submodules super dst 2>err &&
+	test_i18ngrep "sub " err
+'
+
 test_done

t/t9350-fast-export.sh

@@ -424,7 +424,7 @@ test_expect_success 'fast-export quotes pathnames' '
 	test_config -C crazy-paths core.protectNTFS false &&
 	(cd crazy-paths &&
 	 blob=$(echo foo | git hash-object -w --stdin) &&
-	 git update-index --add \
+	 git -c core.protectNTFS=false update-index --add \
 		--cacheinfo 100644 $blob "$(printf "path with\\nnewline")" \
 		--cacheinfo 100644 $blob "path with \"quote\"" \
 		--cacheinfo 100644 $blob "path with \\backslash" \

unpack-trees.c

@@ -1821,7 +1821,8 @@ static int merged_entry(const struct cache_entry *ce,
 		invalidate_ce_path(old, o);
 	}
 
-	do_add_entry(o, merge, update, CE_STAGEMASK);
+	if (do_add_entry(o, merge, update, CE_STAGEMASK) < 0)
+		return -1;
 	return 1;
 }