From ca8a4cd363bff7c482bb434d018610ffaeb213bf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Sep 2019 15:25:02 +0200 Subject: Protect against 8.3 "short name" attacks also on Linux/macOS The Windows Subsystem for Linux (WSL) is getting increasingly popular, in particular because it makes it _so_ easy to run Linux software on Windows' files, via the auto-mounted Windows drives (`C:\` is mapped to `/mnt/c/`, no need to set that up manually). Unfortunately, files/directories on the Windows drives can be accessed via their _short names_, if that feature is enabled (which it is on the `C:` drive by default). Which means that we have to safeguard even our Linux users against the short name attacks. Further, while the default options of CIFS/SMB-mounts seem to disallow accessing files on network shares via their short names on Linux/macOS, it _is_ possible to do so with the right options. So let's just safe-guard against short name attacks _everywhere_. Signed-off-by: Johannes Schindelin --- src/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/checkout.c b/src/checkout.c index debdbe95b..8567010ad 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1273,7 +1273,7 @@ static int checkout_verify_paths( int action, git_diff_delta *delta) { - unsigned int flags = GIT_PATH_REJECT_WORKDIR_DEFAULTS; + unsigned int flags = GIT_PATH_REJECT_WORKDIR_DEFAULTS | GIT_PATH_REJECT_DOT_GIT_NTFS; if (action & CHECKOUT_ACTION__REMOVE) { if (!git_path_isvalid(repo, delta->old_file.path, delta->old_file.mode, flags)) { -- cgit v1.2.1 From 3595e237c98bcae11e655826d12f29361637510d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Sep 2019 14:32:05 +0200 Subject: Disallow NTFS Alternate Data Stream attacks, even on Linux/macOS A little-known feature of NTFS is that it offers to store metadata in so-called "Alternate Data Streams" (inspired by Apple's "resource forks") that are copied together with the file they are associated with. These Alternate Data Streams can be accessed via `::`. Directories, too, have Alternate Data Streams, and they even have a default stream type `$INDEX_ALLOCATION`. Which means that `abc/` and `abc::$INDEX_ALLOCATION/` are actually equivalent. This is of course another attack vector on the Git directory that we definitely want to prevent. On Windows, we already do this incidentally, by disallowing colons in file/directory names. While it looks as if files'/directories' Alternate Data Streams are not accessible in the Windows Subsystem for Linux, and neither via CIFS/SMB-mounted network shares in Linux, it _is_ possible to access them on SMB-mounted network shares on macOS. Therefore, let's go the extra mile and prevent this particular attack _everywhere_. To keep things simple, let's just disallow *any* Alternate Data Stream of `.git`. This is libgit2's variant of CVE-2019-1352. Signed-off-by: Johannes Schindelin --- src/path.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/path.c b/src/path.c index 32cae8e5a..6f22b08e9 100644 --- a/src/path.c +++ b/src/path.c @@ -1609,8 +1609,12 @@ GIT_INLINE(bool) verify_dotgit_ntfs(git_repository *repo, const char *path, size if (!start) return true; - /* Reject paths like ".git\" */ - if (path[start] == '\\') + /* + * Reject paths that start with Windows-style directory separators + * (".git\") or NTFS alternate streams (".git:") and could be used + * to write to the ".git" directory on Windows platforms. + */ + if (path[start] == '\\' || path[start] == ':') return false; /* Reject paths like '.git ' or '.git.' */ -- cgit v1.2.1 From 4bae85c5b9f2c761827b86d3c82385487d60febe Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 18 Sep 2019 16:33:18 +0200 Subject: path: also guard `.gitmodules` against NTFS Alternate Data Streams We just safe-guarded `.git` against NTFS Alternate Data Stream-related attack vectors, and now it is time to do the same for `.gitmodules`. Note: In the added regression test, we refrain from verifying all kinds of variations between short names and NTFS Alternate Data Streams: as the new code disallows _all_ Alternate Data Streams of `.gitmodules`, it is enough to test one in order to know that all of them are guarded against. Signed-off-by: Johannes Schindelin --- src/path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/path.c b/src/path.c index 6f22b08e9..508736532 100644 --- a/src/path.c +++ b/src/path.c @@ -1631,7 +1631,7 @@ GIT_INLINE(bool) only_spaces_and_dots(const char *path) const char *c = path; for (;; c++) { - if (*c == '\0') + if (*c == '\0' || *c == ':') return true; if (*c != ' ' && *c != '.') return false; -- cgit v1.2.1 From 988757feefc77d7aa19f51a7671eb08759943a8d Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 3 Dec 2019 17:47:31 +1100 Subject: path: rename function that detects end of filename The function `only_spaces_and_dots` used to detect the end of the filename on win32. Now we look at spaces and dots _before_ the end of the string _or_ a `:` character, which would signify a win32 alternate data stream. Thus, rename the function `ntfs_end_of_filename` to indicate that it detects the (virtual) end of a filename, that any further characters would be elided to the given path. --- src/path.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/path.c b/src/path.c index 508736532..2b8465f34 100644 --- a/src/path.c +++ b/src/path.c @@ -1626,7 +1626,16 @@ GIT_INLINE(bool) verify_dotgit_ntfs(git_repository *repo, const char *path, size return false; } -GIT_INLINE(bool) only_spaces_and_dots(const char *path) +/* + * Windows paths that end with spaces and/or dots are elided to the + * path without them for backward compatibility. That is to say + * that opening file "foo ", "foo." or even "foo . . ." will all + * map to a filename of "foo". This function identifies spaces and + * dots at the end of a filename, whether the proper end of the + * filename (end of string) or a colon (which would indicate a + * Windows alternate data stream.) + */ +GIT_INLINE(bool) ntfs_end_of_filename(const char *path) { const char *c = path; @@ -1646,13 +1655,13 @@ GIT_INLINE(bool) verify_dotgit_ntfs_generic(const char *name, size_t len, const if (name[0] == '.' && len >= dotgit_len && !strncasecmp(name + 1, dotgit_name, dotgit_len)) { - return !only_spaces_and_dots(name + dotgit_len + 1); + return !ntfs_end_of_filename(name + dotgit_len + 1); } /* Detect the basic NTFS shortname with the first six chars */ if (!strncasecmp(name, dotgit_name, 6) && name[6] == '~' && name[7] >= '1' && name[7] <= '4') - return !only_spaces_and_dots(name + 8); + return !ntfs_end_of_filename(name + 8); /* Catch fallback names */ for (i = 0, saw_tilde = 0; i < 8; i++) { @@ -1674,7 +1683,7 @@ GIT_INLINE(bool) verify_dotgit_ntfs_generic(const char *name, size_t len, const } } - return !only_spaces_and_dots(name + i); + return !ntfs_end_of_filename(name + i); } GIT_INLINE(bool) verify_char(unsigned char c, unsigned int flags) -- cgit v1.2.1 From 31541e5fd3800b82007f70ccd32d1fa078682a2c Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 3 Dec 2019 19:24:59 +1100 Subject: path: protect NTFS everywhere Enable core.protectNTFS by default everywhere and in every codepath, not just on checkout. --- src/checkout.c | 2 +- src/path.c | 8 ++------ src/repository.h | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/checkout.c b/src/checkout.c index 8567010ad..debdbe95b 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -1273,7 +1273,7 @@ static int checkout_verify_paths( int action, git_diff_delta *delta) { - unsigned int flags = GIT_PATH_REJECT_WORKDIR_DEFAULTS | GIT_PATH_REJECT_DOT_GIT_NTFS; + unsigned int flags = GIT_PATH_REJECT_WORKDIR_DEFAULTS; if (action & CHECKOUT_ACTION__REMOVE) { if (!git_path_isvalid(repo, delta->old_file.path, delta->old_file.mode, flags)) { diff --git a/src/path.c b/src/path.c index 2b8465f34..3217fd7ce 100644 --- a/src/path.c +++ b/src/path.c @@ -1817,7 +1817,7 @@ GIT_INLINE(unsigned int) dotgit_flags( git_repository *repo, unsigned int flags) { - int protectHFS = 0, protectNTFS = 0; + int protectHFS = 0, protectNTFS = 1; int error = 0; flags |= GIT_PATH_REJECT_DOT_GIT_LITERAL; @@ -1826,16 +1826,12 @@ GIT_INLINE(unsigned int) dotgit_flags( protectHFS = 1; #endif -#ifdef GIT_WIN32 - protectNTFS = 1; -#endif - if (repo && !protectHFS) error = git_repository__cvar(&protectHFS, repo, GIT_CVAR_PROTECTHFS); if (!error && protectHFS) flags |= GIT_PATH_REJECT_DOT_GIT_HFS; - if (repo && !protectNTFS) + if (repo) error = git_repository__cvar(&protectNTFS, repo, GIT_CVAR_PROTECTNTFS); if (!error && protectNTFS) flags |= GIT_PATH_REJECT_DOT_GIT_NTFS; diff --git a/src/repository.h b/src/repository.h index fd6400cc1..65b38b5d6 100644 --- a/src/repository.h +++ b/src/repository.h @@ -110,7 +110,7 @@ typedef enum { /* core.protectHFS */ GIT_PROTECTHFS_DEFAULT = GIT_CVAR_FALSE, /* core.protectNTFS */ - GIT_PROTECTNTFS_DEFAULT = GIT_CVAR_FALSE, + GIT_PROTECTNTFS_DEFAULT = GIT_CVAR_TRUE, /* core.fsyncObjectFiles */ GIT_FSYNCOBJECTFILES_DEFAULT = GIT_CVAR_FALSE, } git_cvar_value; -- cgit v1.2.1 From 6117dcd34a250a6a69f18cf07c8a1c62459ed694 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 3 Dec 2019 23:15:47 +1100 Subject: path: support non-ascii drive letters on dos Windows/DOS only supports drive letters that are alpha characters A-Z. However, you can `subst` any one-character as a drive letter, including numbers or even emoji. Test that we can identify emoji as drive letters. --- src/path.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/path.c b/src/path.c index 3217fd7ce..6d37abda1 100644 --- a/src/path.c +++ b/src/path.c @@ -21,7 +21,29 @@ #include #include -#define LOOKS_LIKE_DRIVE_PREFIX(S) (git__isalpha((S)[0]) && (S)[1] == ':') +static int dos_drive_prefix_length(const char *path) +{ + int i; + + /* + * Does it start with an ASCII letter (i.e. highest bit not set), + * followed by a colon? + */ + if (!(0x80 & (unsigned char)*path)) + return *path && path[1] == ':' ? 2 : 0; + + /* + * While drive letters must be letters of the English alphabet, it is + * possible to assign virtually _any_ Unicode character via `subst` as + * a drive letter to "virtual drives". Even `1`, or `ä`. Or fun stuff + * like this: + * + * subst ֍: %USERPROFILE%\Desktop + */ + for (i = 1; i < 4 && (0x80 & (unsigned char)path[i]); i++) + ; /* skip first UTF-8 character */ + return path[i] == ':' ? i + 1 : 0; +} #ifdef GIT_WIN32 static bool looks_like_network_computer_name(const char *path, int pos) @@ -123,11 +145,11 @@ static int win32_prefix_length(const char *path, int len) GIT_UNUSED(len); #else /* - * Mimic unix behavior where '/.git' returns '/': 'C:/.git' will return - * 'C:/' here + * Mimic unix behavior where '/.git' returns '/': 'C:/.git' + * will return 'C:/' here */ - if (len == 2 && LOOKS_LIKE_DRIVE_PREFIX(path)) - return 2; + if (dos_drive_prefix_length(path) == len) + return len; /* * Similarly checks if we're dealing with a network computer name @@ -260,11 +282,11 @@ const char *git_path_topdir(const char *path) int git_path_root(const char *path) { - int offset = 0; + int offset = 0, prefix_len; /* Does the root of the path look like a windows drive ? */ - if (LOOKS_LIKE_DRIVE_PREFIX(path)) - offset += 2; + if ((prefix_len = dos_drive_prefix_length(path))) + offset += prefix_len; #ifdef GIT_WIN32 /* Are we dealing with a windows network path? */ -- cgit v1.2.1