[PATCH 0/2] Refuse to write to reserved filenames on Windows

[PATCH 0/2] Refuse to write to reserved filenames on Windows

[Johannes Schindelin via GitGitGadget]

On Windows, for historical reasons, file names such as aux.c, nul.txt are
not allowed. For aux.c, attempts to write such a file will result in an
obscure error, for nul.txt the call will succeed but no such file will
appear, ever, instead the effect will be equivalent to writing to /dev/null 
on Linux/Unix.

Let's help users by refusing to create such files altogether, with an
informative error message.

Johannes Schindelin (2):
  mingw: short-circuit the conversion of `/dev/null` to UTF-16
  mingw: refuse paths containing reserved names

 compat/mingw.c        | 122 +++++++++++++++++++++++++++++++++++-------
 compat/mingw.h        |  11 +++-
 t/t0060-path-utils.sh |  13 ++++-
 3 files changed, 122 insertions(+), 24 deletions(-)


base-commit: 53a06cf39b756eddfe4a2a34da93e3d04eb7b728
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-496%2Fdscho%2Fmingw-reserved-filenames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-496/dscho/mingw-reserved-filenames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/496
-- 
gitgitgadget

[PATCH 1/2] mingw: short-circuit the conversion of `/dev/null` to UTF-16

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

In the next commit, we want to disallow accessing any path that contains
any segment that is equivalent to `NUL`. In particular, we want to
disallow accessing `NUL` (e.g. to prevent any repository from being
checked out that contains a file called `NUL`, as that is not a valid
file name on Windows).

However, there are legitimate use cases within Git itself to write to
the Null device. As Git is really a Linux project, it does not abstract
that idea, though, but instead uses `/dev/null` to describe this
intention.

So let's side-step the validation _specifically_ in the case that we
want to write to (or read from) `/dev/null`, via a dedicated short-cut
in the code that skips the call to `validate_win32_path()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index bd24d913f9..03c4538ec8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -484,16 +484,16 @@ int mingw_open (const char *filename, int oflags, ...)
 		return -1;
 	}
 
-	if (filename && !strcmp(filename, "/dev/null"))
-		filename = "nul";
-
 	if ((oflags & O_APPEND) && !is_local_named_pipe_path(filename))
 		open_fn = mingw_open_append;
 	else
 		open_fn = _wopen;
 
-	if (xutftowcs_path(wfilename, filename) < 0)
+	if (filename && !strcmp(filename, "/dev/null"))
+		wcscpy(wfilename, L"nul");
+	else if (xutftowcs_path(wfilename, filename) < 0)
 		return -1;
+
 	fd = open_fn(wfilename, oflags, mode);
 
 	if (fd < 0 && (oflags & O_ACCMODE) != O_RDONLY && errno == EACCES) {
@@ -556,10 +556,13 @@ FILE *mingw_fopen (const char *filename, const char *otype)
 		return NULL;
 	}
 	if (filename && !strcmp(filename, "/dev/null"))
-		filename = "nul";
-	if (xutftowcs_path(wfilename, filename) < 0 ||
-		xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
+		wcscpy(wfilename, L"nul");
+	else if (xutftowcs_path(wfilename, filename) < 0)
 		return NULL;
+
+	if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
+		return NULL;
+
 	if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) {
 		error("could not unhide %s", filename);
 		return NULL;
@@ -583,10 +586,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
 		return NULL;
 	}
 	if (filename && !strcmp(filename, "/dev/null"))
-		filename = "nul";
-	if (xutftowcs_path(wfilename, filename) < 0 ||
-		xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
+		wcscpy(wfilename, L"nul");
+	else if (xutftowcs_path(wfilename, filename) < 0)
 		return NULL;
+
+	if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
+		return NULL;
+
 	if (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) {
 		error("could not unhide %s", filename);
 		return NULL;
-- 
gitgitgadget

[PATCH 2/2] mingw: refuse paths containing reserved names

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

There are a couple of reserved names that cannot be file names on
Windows, such as `AUX`, `NUL`, etc. For an almost complete list, see
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

If one would try to create a directory named `NUL`, it would actually
"succeed", i.e. the call would return success, but nothing would be
created.

Worse, even adding a file extension to the reserved name does not make
it a valid file name. To understand the rationale behind that behavior,
see https://devblogs.microsoft.com/oldnewthing/20031022-00/?p=42073

Let's just disallow them all.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c        | 104 ++++++++++++++++++++++++++++++++++++------
 compat/mingw.h        |  11 ++++-
 t/t0060-path-utils.sh |  13 +++++-
 3 files changed, 110 insertions(+), 18 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 03c4538ec8..f482ecd6da 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -393,7 +393,7 @@ int mingw_mkdir(const char *path, int mode)
 	int ret;
 	wchar_t wpath[MAX_PATH];
 
-	if (!is_valid_win32_path(path)) {
+	if (!is_valid_win32_path(path, 0)) {
 		errno = EINVAL;
 		return -1;
 	}
@@ -479,7 +479,7 @@ int mingw_open (const char *filename, int oflags, ...)
 	mode = va_arg(args, int);
 	va_end(args);
 
-	if (!is_valid_win32_path(filename)) {
+	if (!is_valid_win32_path(filename, !create)) {
 		errno = create ? EINVAL : ENOENT;
 		return -1;
 	}
@@ -550,14 +550,13 @@ 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)) {
+	if (filename && !strcmp(filename, "/dev/null"))
+		wcscpy(wfilename, L"nul");
+	else if (!is_valid_win32_path(filename, 1)) {
 		int create = otype && strchr(otype, 'w');
 		errno = create ? EINVAL : ENOENT;
 		return NULL;
-	}
-	if (filename && !strcmp(filename, "/dev/null"))
-		wcscpy(wfilename, L"nul");
-	else if (xutftowcs_path(wfilename, filename) < 0)
+	} else if (xutftowcs_path(wfilename, filename) < 0)
 		return NULL;
 
 	if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
@@ -580,14 +579,13 @@ 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)) {
+	if (filename && !strcmp(filename, "/dev/null"))
+		wcscpy(wfilename, L"nul");
+	else if (!is_valid_win32_path(filename, 1)) {
 		int create = otype && strchr(otype, 'w');
 		errno = create ? EINVAL : ENOENT;
 		return NULL;
-	}
-	if (filename && !strcmp(filename, "/dev/null"))
-		wcscpy(wfilename, L"nul");
-	else if (xutftowcs_path(wfilename, filename) < 0)
+	} else if (xutftowcs_path(wfilename, filename) < 0)
 		return NULL;
 
 	if (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)
@@ -2412,14 +2410,16 @@ static void setup_windows_environment(void)
 	}
 }
 
-int is_valid_win32_path(const char *path)
+int is_valid_win32_path(const char *path, int allow_literal_nul)
 {
+	const char *p = path;
 	int preceding_space_or_period = 0, i = 0, periods = 0;
 
 	if (!protect_ntfs)
 		return 1;
 
 	skip_dos_drive_prefix((char **)&path);
+	goto segment_start;
 
 	for (;;) {
 		char c = *(path++);
@@ -2434,7 +2434,83 @@ int is_valid_win32_path(const char *path)
 				return 1;
 
 			i = periods = preceding_space_or_period = 0;
-			continue;
+
+segment_start:
+			switch (*path) {
+			case 'a': case 'A': /* AUX */
+				if (((c = path[++i]) != 'u' && c != 'U') ||
+				    ((c = path[++i]) != 'x' && c != 'X')) {
+not_a_reserved_name:
+					path += i;
+					continue;
+				}
+				break;
+			case 'c': case 'C': /* COM<N>, CON, CONIN$, CONOUT$ */
+				if ((c = path[++i]) != 'o' && c != 'O')
+					goto not_a_reserved_name;
+				c = path[++i];
+				if (c == 'm' || c == 'M') { /* COM<N> */
+					if (!isdigit(path[++i]))
+						goto not_a_reserved_name;
+				} else if (c == 'n' || c == 'N') { /* CON */
+					c = path[i + 1];
+					if ((c == 'i' || c == 'I') &&
+					    ((c = path[i + 2]) == 'n' ||
+					     c == 'N') &&
+					    path[i + 3] == '$')
+						i += 3; /* CONIN$ */
+					else if ((c == 'o' || c == 'O') &&
+						 ((c = path[i + 2]) == 'u' ||
+						  c == 'U') &&
+						 ((c = path[i + 3]) == 't' ||
+						  c == 'T') &&
+						 path[i + 4] == '$')
+						i += 4; /* CONOUT$ */
+				} else
+					goto not_a_reserved_name;
+				break;
+			case 'l': case 'L': /* LPT<N> */
+				if (((c = path[++i]) != 'p' && c != 'P') ||
+				    ((c = path[++i]) != 't' && c != 'T') ||
+				    !isdigit(path[++i]))
+					goto not_a_reserved_name;
+				break;
+			case 'n': case 'N': /* NUL */
+				if (((c = path[++i]) != 'u' && c != 'U') ||
+				    ((c = path[++i]) != 'l' && c != 'L') ||
+				    (allow_literal_nul &&
+				     !path[i + 1] && p == path))
+					goto not_a_reserved_name;
+				break;
+			case 'p': case 'P': /* PRN */
+				if (((c = path[++i]) != 'r' && c != 'R') ||
+				    ((c = path[++i]) != 'n' && c != 'N'))
+					goto not_a_reserved_name;
+				break;
+			default:
+				continue;
+			}
+
+			/*
+			 * So far, this looks like a reserved name. Let's see
+			 * whether it actually is one: trailing spaces, a file
+			 * extension, or an NTFS Alternate Data Stream do not
+			 * matter, the name is still reserved if any of those
+			 * follow immediately after the actual name.
+			 */
+			i++;
+			if (path[i] == ' ') {
+				preceding_space_or_period = 1;
+				while (path[++i] == ' ')
+					; /* skip all spaces */
+			}
+
+			c = path[i];
+			if (c && c != '.' && c != ':' && c != '/' && c != '\\')
+				goto not_a_reserved_name;
+
+			/* contains reserved name */
+			return 0;
 		case '.':
 			periods++;
 			/* fallthru */
diff --git a/compat/mingw.h b/compat/mingw.h
index 04ca731a6b..ebc1e6a773 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -461,10 +461,17 @@ char *mingw_query_user_email(void);
  *
  * - contain any of the reserved characters, e.g. `:`, `;`, `*`, etc
  *
+ * - correspond to reserved names (such as `AUX`, `PRN`, etc)
+ *
+ * The `allow_literal_nul` parameter controls whether the path `NUL` should
+ * be considered valid (this makes sense e.g. before opening files, as it is
+ * perfectly legitimate to open `NUL` on Windows, just as it is to open
+ * `/dev/null` on Unix/Linux).
+ *
  * Returns 1 upon success, otherwise 0.
  */
-int is_valid_win32_path(const char *path);
-#define is_valid_path(path) is_valid_win32_path(path)
+int is_valid_win32_path(const char *path, int allow_literal_nul);
+#define is_valid_path(path) is_valid_win32_path(path, 0)
 
 /**
  * Converts UTF-8 encoded string to UTF-16LE.
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index b193ed4205..eaf3be94e3 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -465,11 +465,14 @@ test_expect_success 'match .gitmodules' '
 '
 
 test_expect_success MINGW 'is_valid_path() on Windows' '
-       test-tool path-utils is_valid_path \
+	test-tool path-utils is_valid_path \
 		win32 \
 		"win32 x" \
 		../hello.txt \
 		C:\\git \
+		comm \
+		conout.c \
+		lptN \
 		\
 		--not \
 		"win32 "  \
@@ -477,7 +480,13 @@ test_expect_success MINGW 'is_valid_path() on Windows' '
 		"win32."  \
 		"win32 . ." \
 		.../hello.txt \
-		colon:test
+		colon:test \
+		"AUX.c" \
+		"abc/conOut\$  .xyz/test" \
+		lpt8 \
+		"lpt*" \
+		Nul \
+		"PRN./abc"
 '
 
 test_done
-- 
gitgitgadget