{"thread":{"id":"52504","subject":"[PATCH 0/2] Refuse to write to reserved filenames on Windows","startedAt":"2019-12-21T22:05:05Z","lastAt":"2019-12-21T22:05:28Z","messageCount":3,"participants":["Johannes Schindelin via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"388751","messageId":"pull.496.git.1576965901.gitgitgadget@gmail.com","threadId":"52504","inReplyTo":null,"subject":"[PATCH 0/2] Refuse to write to reserved filenames on Windows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:04:59Z","receivedAt":"2019-12-21T22:05:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"On Windows, for historical reasons, file names such as aux.c, nul.txt are\nnot allowed. For aux.c, attempts to write such a file will result in an\nobscure error, for nul.txt the call will succeed but no such file will\nappear, ever, instead the effect will be equivalent to writing to /dev/null \non Linux/Unix.\n\nLet's help users by refusing to create such files altogether, with an\ninformative error message.\n\nJohannes Schindelin (2):\n  mingw: short-circuit the conversion of `/dev/null` to UTF-16\n  mingw: refuse paths containing reserved names\n\n compat/mingw.c        | 122 +++++++++++++++++++++++++++++++++++-------\n compat/mingw.h        |  11 +++-\n t/t0060-path-utils.sh |  13 ++++-\n 3 files changed, 122 insertions(+), 24 deletions(-)\n\n\nbase-commit: 53a06cf39b756eddfe4a2a34da93e3d04eb7b728\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-496%2Fdscho%2Fmingw-reserved-filenames-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-496/dscho/mingw-reserved-filenames-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/496\n-- \ngitgitgadget\n"},{"id":"388752","messageId":"171871addc00acb12a77796c97d7f7b4f605ebc9.1576965901.git.gitgitgadget@gmail.com","threadId":"52504","inReplyTo":"pull.496.git.1576965901.gitgitgadget@gmail.com","subject":"[PATCH 2/2] mingw: refuse paths containing reserved names","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:05:01Z","receivedAt":"2019-12-21T22:05:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThere are a couple of reserved names that cannot be file names on\nWindows, such as `AUX`, `NUL`, etc. For an almost complete list, see\nhttps://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file\n\nIf one would try to create a directory named `NUL`, it would actually\n\"succeed\", i.e. the call would return success, but nothing would be\ncreated.\n\nWorse, even adding a file extension to the reserved name does not make\nit a valid file name. To understand the rationale behind that behavior,\nsee https://devblogs.microsoft.com/oldnewthing/20031022-00/?p=42073\n\nLet's just disallow them all.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c        | 104 ++++++++++++++++++++++++++++++++++++------\n compat/mingw.h        |  11 ++++-\n t/t0060-path-utils.sh |  13 +++++-\n 3 files changed, 110 insertions(+), 18 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 03c4538ec8..f482ecd6da 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -393,7 +393,7 @@ int mingw_mkdir(const char *path, int mode)\n \tint ret;\n \twchar_t wpath[MAX_PATH];\n \n-\tif (!is_valid_win32_path(path)) {\n+\tif (!is_valid_win32_path(path, 0)) {\n \t\terrno = EINVAL;\n \t\treturn -1;\n \t}\n@@ -479,7 +479,7 @@ int mingw_open (const char *filename, int oflags, ...)\n \tmode = va_arg(args, int);\n \tva_end(args);\n \n-\tif (!is_valid_win32_path(filename)) {\n+\tif (!is_valid_win32_path(filename, !create)) {\n \t\terrno = create ? EINVAL : ENOENT;\n \t\treturn -1;\n \t}\n@@ -550,14 +550,13 @@ FILE *mingw_fopen (const char *filename, const char *otype)\n \tint hide = needs_hiding(filename);\n \tFILE *file;\n \twchar_t wfilename[MAX_PATH], wotype[4];\n-\tif (!is_valid_win32_path(filename)) {\n+\tif (filename && !strcmp(filename, \"/dev/null\"))\n+\t\twcscpy(wfilename, L\"nul\");\n+\telse if (!is_valid_win32_path(filename, 1)) {\n \t\tint create = otype && strchr(otype, 'w');\n \t\terrno = create ? EINVAL : ENOENT;\n \t\treturn NULL;\n-\t}\n-\tif (filename && !strcmp(filename, \"/dev/null\"))\n-\t\twcscpy(wfilename, L\"nul\");\n-\telse if (xutftowcs_path(wfilename, filename) < 0)\n+\t} else if (xutftowcs_path(wfilename, filename) < 0)\n \t\treturn NULL;\n \n \tif (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n@@ -580,14 +579,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)\n \tint hide = needs_hiding(filename);\n \tFILE *file;\n \twchar_t wfilename[MAX_PATH], wotype[4];\n-\tif (!is_valid_win32_path(filename)) {\n+\tif (filename && !strcmp(filename, \"/dev/null\"))\n+\t\twcscpy(wfilename, L\"nul\");\n+\telse if (!is_valid_win32_path(filename, 1)) {\n \t\tint create = otype && strchr(otype, 'w');\n \t\terrno = create ? EINVAL : ENOENT;\n \t\treturn NULL;\n-\t}\n-\tif (filename && !strcmp(filename, \"/dev/null\"))\n-\t\twcscpy(wfilename, L\"nul\");\n-\telse if (xutftowcs_path(wfilename, filename) < 0)\n+\t} else if (xutftowcs_path(wfilename, filename) < 0)\n \t\treturn NULL;\n \n \tif (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n@@ -2412,14 +2410,16 @@ static void setup_windows_environment(void)\n \t}\n }\n \n-int is_valid_win32_path(const char *path)\n+int is_valid_win32_path(const char *path, int allow_literal_nul)\n {\n+\tconst char *p = path;\n \tint preceding_space_or_period = 0, i = 0, periods = 0;\n \n \tif (!protect_ntfs)\n \t\treturn 1;\n \n \tskip_dos_drive_prefix((char **)&path);\n+\tgoto segment_start;\n \n \tfor (;;) {\n \t\tchar c = *(path++);\n@@ -2434,7 +2434,83 @@ int is_valid_win32_path(const char *path)\n \t\t\t\treturn 1;\n \n \t\t\ti = periods = preceding_space_or_period = 0;\n-\t\t\tcontinue;\n+\n+segment_start:\n+\t\t\tswitch (*path) {\n+\t\t\tcase 'a': case 'A': /* AUX */\n+\t\t\t\tif (((c = path[++i]) != 'u' && c != 'U') ||\n+\t\t\t\t    ((c = path[++i]) != 'x' && c != 'X')) {\n+not_a_reserved_name:\n+\t\t\t\t\tpath += i;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase 'c': case 'C': /* COM<N>, CON, CONIN$, CONOUT$ */\n+\t\t\t\tif ((c = path[++i]) != 'o' && c != 'O')\n+\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\tc = path[++i];\n+\t\t\t\tif (c == 'm' || c == 'M') { /* COM<N> */\n+\t\t\t\t\tif (!isdigit(path[++i]))\n+\t\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\t} else if (c == 'n' || c == 'N') { /* CON */\n+\t\t\t\t\tc = path[i + 1];\n+\t\t\t\t\tif ((c == 'i' || c == 'I') &&\n+\t\t\t\t\t    ((c = path[i + 2]) == 'n' ||\n+\t\t\t\t\t     c == 'N') &&\n+\t\t\t\t\t    path[i + 3] == '$')\n+\t\t\t\t\t\ti += 3; /* CONIN$ */\n+\t\t\t\t\telse if ((c == 'o' || c == 'O') &&\n+\t\t\t\t\t\t ((c = path[i + 2]) == 'u' ||\n+\t\t\t\t\t\t  c == 'U') &&\n+\t\t\t\t\t\t ((c = path[i + 3]) == 't' ||\n+\t\t\t\t\t\t  c == 'T') &&\n+\t\t\t\t\t\t path[i + 4] == '$')\n+\t\t\t\t\t\ti += 4; /* CONOUT$ */\n+\t\t\t\t} else\n+\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\tbreak;\n+\t\t\tcase 'l': case 'L': /* LPT<N> */\n+\t\t\t\tif (((c = path[++i]) != 'p' && c != 'P') ||\n+\t\t\t\t    ((c = path[++i]) != 't' && c != 'T') ||\n+\t\t\t\t    !isdigit(path[++i]))\n+\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\tbreak;\n+\t\t\tcase 'n': case 'N': /* NUL */\n+\t\t\t\tif (((c = path[++i]) != 'u' && c != 'U') ||\n+\t\t\t\t    ((c = path[++i]) != 'l' && c != 'L') ||\n+\t\t\t\t    (allow_literal_nul &&\n+\t\t\t\t     !path[i + 1] && p == path))\n+\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\tbreak;\n+\t\t\tcase 'p': case 'P': /* PRN */\n+\t\t\t\tif (((c = path[++i]) != 'r' && c != 'R') ||\n+\t\t\t\t    ((c = path[++i]) != 'n' && c != 'N'))\n+\t\t\t\t\tgoto not_a_reserved_name;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\t/*\n+\t\t\t * So far, this looks like a reserved name. Let's see\n+\t\t\t * whether it actually is one: trailing spaces, a file\n+\t\t\t * extension, or an NTFS Alternate Data Stream do not\n+\t\t\t * matter, the name is still reserved if any of those\n+\t\t\t * follow immediately after the actual name.\n+\t\t\t */\n+\t\t\ti++;\n+\t\t\tif (path[i] == ' ') {\n+\t\t\t\tpreceding_space_or_period = 1;\n+\t\t\t\twhile (path[++i] == ' ')\n+\t\t\t\t\t; /* skip all spaces */\n+\t\t\t}\n+\n+\t\t\tc = path[i];\n+\t\t\tif (c && c != '.' && c != ':' && c != '/' && c != '\\\\')\n+\t\t\t\tgoto not_a_reserved_name;\n+\n+\t\t\t/* contains reserved name */\n+\t\t\treturn 0;\n \t\tcase '.':\n \t\t\tperiods++;\n \t\t\t/* fallthru */\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 04ca731a6b..ebc1e6a773 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -461,10 +461,17 @@ char *mingw_query_user_email(void);\n  *\n  * - contain any of the reserved characters, e.g. `:`, `;`, `*`, etc\n  *\n+ * - correspond to reserved names (such as `AUX`, `PRN`, etc)\n+ *\n+ * The `allow_literal_nul` parameter controls whether the path `NUL` should\n+ * be considered valid (this makes sense e.g. before opening files, as it is\n+ * perfectly legitimate to open `NUL` on Windows, just as it is to open\n+ * `/dev/null` on Unix/Linux).\n+ *\n  * Returns 1 upon success, otherwise 0.\n  */\n-int is_valid_win32_path(const char *path);\n-#define is_valid_path(path) is_valid_win32_path(path)\n+int is_valid_win32_path(const char *path, int allow_literal_nul);\n+#define is_valid_path(path) is_valid_win32_path(path, 0)\n \n /**\n  * Converts UTF-8 encoded string to UTF-16LE.\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex b193ed4205..eaf3be94e3 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -465,11 +465,14 @@ test_expect_success 'match .gitmodules' '\n '\n \n test_expect_success MINGW 'is_valid_path() on Windows' '\n-       test-tool path-utils is_valid_path \\\n+\ttest-tool path-utils is_valid_path \\\n \t\twin32 \\\n \t\t\"win32 x\" \\\n \t\t../hello.txt \\\n \t\tC:\\\\git \\\n+\t\tcomm \\\n+\t\tconout.c \\\n+\t\tlptN \\\n \t\t\\\n \t\t--not \\\n \t\t\"win32 \"  \\\n@@ -477,7 +480,13 @@ test_expect_success MINGW 'is_valid_path() on Windows' '\n \t\t\"win32.\"  \\\n \t\t\"win32 . .\" \\\n \t\t.../hello.txt \\\n-\t\tcolon:test\n+\t\tcolon:test \\\n+\t\t\"AUX.c\" \\\n+\t\t\"abc/conOut\\$  .xyz/test\" \\\n+\t\tlpt8 \\\n+\t\t\"lpt*\" \\\n+\t\tNul \\\n+\t\t\"PRN./abc\"\n '\n \n test_done\n-- \ngitgitgadget\n"},{"id":"388753","messageId":"f4dbaa66cc313d458f1a15c0c258016abfc698a0.1576965901.git.gitgitgadget@gmail.com","threadId":"52504","inReplyTo":"pull.496.git.1576965901.gitgitgadget@gmail.com","subject":"[PATCH 1/2] mingw: short-circuit the conversion of `/dev/null` to UTF-16","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-21T22:05:00Z","receivedAt":"2019-12-21T22:05:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the next commit, we want to disallow accessing any path that contains\nany segment that is equivalent to `NUL`. In particular, we want to\ndisallow accessing `NUL` (e.g. to prevent any repository from being\nchecked out that contains a file called `NUL`, as that is not a valid\nfile name on Windows).\n\nHowever, there are legitimate use cases within Git itself to write to\nthe Null device. As Git is really a Linux project, it does not abstract\nthat idea, though, but instead uses `/dev/null` to describe this\nintention.\n\nSo let's side-step the validation _specifically_ in the case that we\nwant to write to (or read from) `/dev/null`, via a dedicated short-cut\nin the code that skips the call to `validate_win32_path()`.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n compat/mingw.c | 26 ++++++++++++++++----------\n 1 file changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex bd24d913f9..03c4538ec8 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -484,16 +484,16 @@ int mingw_open (const char *filename, int oflags, ...)\n \t\treturn -1;\n \t}\n \n-\tif (filename && !strcmp(filename, \"/dev/null\"))\n-\t\tfilename = \"nul\";\n-\n \tif ((oflags & O_APPEND) && !is_local_named_pipe_path(filename))\n \t\topen_fn = mingw_open_append;\n \telse\n \t\topen_fn = _wopen;\n \n-\tif (xutftowcs_path(wfilename, filename) < 0)\n+\tif (filename && !strcmp(filename, \"/dev/null\"))\n+\t\twcscpy(wfilename, L\"nul\");\n+\telse if (xutftowcs_path(wfilename, filename) < 0)\n \t\treturn -1;\n+\n \tfd = open_fn(wfilename, oflags, mode);\n \n \tif (fd < 0 && (oflags & O_ACCMODE) != O_RDONLY && errno == EACCES) {\n@@ -556,10 +556,13 @@ FILE *mingw_fopen (const char *filename, const char *otype)\n \t\treturn NULL;\n \t}\n \tif (filename && !strcmp(filename, \"/dev/null\"))\n-\t\tfilename = \"nul\";\n-\tif (xutftowcs_path(wfilename, filename) < 0 ||\n-\t\txutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n+\t\twcscpy(wfilename, L\"nul\");\n+\telse if (xutftowcs_path(wfilename, filename) < 0)\n \t\treturn NULL;\n+\n+\tif (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n+\t\treturn NULL;\n+\n \tif (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) {\n \t\terror(\"could not unhide %s\", filename);\n \t\treturn NULL;\n@@ -583,10 +586,13 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)\n \t\treturn NULL;\n \t}\n \tif (filename && !strcmp(filename, \"/dev/null\"))\n-\t\tfilename = \"nul\";\n-\tif (xutftowcs_path(wfilename, filename) < 0 ||\n-\t\txutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n+\t\twcscpy(wfilename, L\"nul\");\n+\telse if (xutftowcs_path(wfilename, filename) < 0)\n \t\treturn NULL;\n+\n+\tif (xutftowcs(wotype, otype, ARRAY_SIZE(wotype)) < 0)\n+\t\treturn NULL;\n+\n \tif (hide && !access(filename, F_OK) && set_hidden_flag(wfilename, 0)) {\n \t\terror(\"could not unhide %s\", filename);\n \t\treturn NULL;\n-- \ngitgitgadget\n\n"}]}