mingw: refuse to access paths with illegal characters

Certain characters are not admissible in file names on Windows, even if
Cygwin/MSYS2 (and therefore, Git for Windows' Bash) pretend that they
are, e.g. `:`, `<`, `>`, etc

Let's disallow those characters explicitly in Windows builds of Git.

Note: just like trailing spaces or periods, it _is_ possible on Windows
to create commits adding files with such illegal characters, as long as
the operation leaves the worktree untouched. To allow for that, we
continue to guard `is_valid_win32_path()` behind the config setting
`core.protectNTFS`, so that users _can_ continue to do that, as long as
they turn the protections off via that config setting.

Among other problems, this prevents Git from trying to write to an "NTFS
Alternate Data Stream" (which refers to metadata stored alongside a
file, under a special name: "<filename>:<stream-name>"). This fix
therefore also prevents an attack vector that was exploited in
demonstrations of a number of recently-fixed security bugs.

Further reading on illegal characters in Win32 filenames:
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file

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

diff --git a/compat/mingw.c b/compat/mingw.c
index 17b4da16e85cef21a26a3674165e67b78ea95ebe..3aea26982d6a04bcdd642fe328fd2cde28ba1c0b 100644 (file)
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2134,6 +2134,8 @@ int is_valid_win32_path(const char *path)
        if (!protect_ntfs)
                return 1;
 
+       skip_dos_drive_prefix((char **)&path);
+
        for (;;) {
                char c = *(path++);
                switch (c) {
@@ -2155,6 +2157,14 @@ int is_valid_win32_path(const char *path)
                        preceding_space_or_period = 1;
                        i++;
                        continue;
+               case ':': /* DOS drive prefix was already skipped */
+               case '<': case '>': case '"': case '|': case '?': case '*':
+                       /* illegal character */
+                       return 0;
+               default:
+                       if (c > '\0' && c < '\x20')
+                               /* illegal character */
+                               return 0;
                }
                preceding_space_or_period = 0;
                i++;

diff --git a/compat/mingw.h b/compat/mingw.h
index 8c49c1d09b697d009e4dc964bc34cfc5377334d7..7482f196afdd2741658c2bd28a2e7eb0d740d3dd 100644 (file)
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -431,8 +431,11 @@ int mingw_offset_1st_component(const char *path);
 /**
  * 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 `..`).
+ * In particular, path segments are disallowed which
+ *
+ * - end in a period or a space (except the special directories `.` and `..`).
+ *
+ * - contain any of the reserved characters, e.g. `:`, `;`, `*`, etc
  *
  * Returns 1 upon success, otherwise 0.
  */

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 1171e0bb8850b97bd3ad0b6e70ce178467e28f65..f7e2529bffc143d15af01d6700fa4953e7d2d3b2 100755 (executable)
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -445,13 +445,15 @@ test_expect_success MINGW 'is_valid_path() on Windows' '
                win32 \
                "win32 x" \
                ../hello.txt \
+               C:\\git \
                \
                --not \
                "win32 "  \
                "win32 /x "  \
                "win32."  \
                "win32 . ." \
-               .../hello.txt
+               .../hello.txt \
+               colon:test
 '
 
 test_done