Skip to contentSkip to conversation

Refuse to write to reserved filenames on Windows

PATCH New Johannes Schindelin via GitGitGadget 1 participant 3 messages Dec 21, 2019

lore
Share & tools
lore
#0 lore raw

cover Refuse to write to reserved filenames on Windows

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

Signature · 1 line
gitgitgadget
#1 lore raw

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

From: Johannes Schindelin <[email protected]>

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().

---
 compat/mingw.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)
compat/mingw.c +16 -10
index bd24d913f9..03c4538ec8 100644
@@ -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;
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/2 mingw: refuse paths containing reserved names

In reply to Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

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.

---
 compat/mingw.c        | 104 ++++++++++++++++++++++++++++++++++++------
 compat/mingw.h        |  11 ++++-
 t/t0060-path-utils.sh |  13 +++++-
 3 files changed, 110 insertions(+), 18 deletions(-)
compat/mingw.c +90 -14
index 03c4538ec8..f482ecd6da 100644
@@ -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 */
compat/mingw.h +9 -2
index 04ca731a6b..ebc1e6a773 100644
@@ -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.
t/t0060-path-utils.sh +11 -2
index b193ed4205..eaf3be94e3 100755
@@ -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
Signature · 1 line
gitgitgadget