143 lines
6.1 KiB
Diff
143 lines
6.1 KiB
Diff
From 9102f958ee5254b10c0be72672aa3305bf4f4704 Mon Sep 17 00:00:00 2001
|
|
From: Johannes Schindelin <johannes.schindelin@gmx.de>
|
|
Date: Mon, 9 Sep 2019 21:04:41 +0200
|
|
Subject: [PATCH] protect_ntfs: turn on NTFS protection by default
|
|
|
|
Back in the DOS days, in the FAT file system, file names always
|
|
consisted of a base name of length 8 plus a file extension of length 3.
|
|
Shorter file names were simply padded with spaces to the full 8.3
|
|
format.
|
|
|
|
Later, the FAT file system was taught to support _also_ longer names,
|
|
with an 8.3 "short name" as primary file name. While at it, the same
|
|
facility allowed formerly illegal file names, such as `.git` (empty base
|
|
names were not allowed), which would have the "short name" `git~1`
|
|
associated with it.
|
|
|
|
For backwards-compatibility, NTFS supports alternative 8.3 short
|
|
filenames, too, even if starting with Windows Vista, they are only
|
|
generated on the system drive by default.
|
|
|
|
We addressed the problem that the `.git/` directory can _also_ be
|
|
accessed via `git~1/` (when short names are enabled) in 2b4c6efc821
|
|
(read-cache: optionally disallow NTFS .git variants, 2014-12-16), i.e.
|
|
since Git v1.9.5, by introducing the config setting `core.protectNTFS`
|
|
and enabling it by default on Windows.
|
|
|
|
In the meantime, Windows 10 introduced the "Windows Subsystem for Linux"
|
|
(short: WSL), i.e. a way to run Linux applications/distributions in a
|
|
thinly-isolated subsystem on Windows (giving rise to many a "2016 is the
|
|
Year of Linux on the Desktop" jokes). WSL is getting increasingly
|
|
popular, also due to the painless way Linux application can operate
|
|
directly ("natively") on files on Windows' file system: the Windows
|
|
drives are mounted automatically (e.g. `C:` as `/mnt/c/`).
|
|
|
|
Taken together, this means that we now have to enable the safe-guards of
|
|
Git v1.9.5 also in WSL: it is possible to access a `.git` directory
|
|
inside `/mnt/c/` via the 8.3 name `git~1` (unless short name generation
|
|
was disabled manually). Since regular Linux distributions run in WSL,
|
|
this means we have to enable `core.protectNTFS` at least on Linux, too.
|
|
|
|
To enable Services for Macintosh in Windows NT to store so-called
|
|
resource forks, NTFS introduced "Alternate Data Streams". Essentially,
|
|
these constitute additional metadata that are connected to (and copied
|
|
with) their associated files, and they are accessed via pseudo file
|
|
names of the form `filename:<stream-name>:<stream-type>`.
|
|
|
|
In a recent patch, we extended `core.protectNTFS` to also protect
|
|
against accesses via NTFS Alternate Data Streams, e.g. to prevent
|
|
contents of the `.git/` directory to be "tracked" via yet another
|
|
alternative file name.
|
|
|
|
While it is not possible (at least by default) to access files via NTFS
|
|
Alternate Data Streams from within WSL, the defaults on macOS when
|
|
mounting network shares via SMB _do_ allow accessing files and
|
|
directories in that way. Therefore, we need to enable `core.protectNTFS`
|
|
on macOS by default, too, and really, on any Operating System that can
|
|
mount network shares via SMB/CIFS.
|
|
|
|
A couple of approaches were considered for fixing this:
|
|
|
|
1. We could perform a dynamic NTFS check similar to the `core.symlinks`
|
|
check in `init`/`clone`: instead of trying to create a symbolic link
|
|
in the `.git/` directory, we could create a test file and try to
|
|
access `.git/config` via 8.3 name and/or Alternate Data Stream.
|
|
|
|
2. We could simply "flip the switch" on `core.protectNTFS`, to make it
|
|
"on by default".
|
|
|
|
The obvious downside of 1. is that it won't protect worktrees that were
|
|
clone with a vulnerable Git version already. We considered patching code
|
|
paths that check out files to check whether we're running on an NTFS
|
|
system dynamically and persist the result in the repository-local config
|
|
setting `core.protectNTFS`, but in the end decided that this solution
|
|
would be too fragile, and too involved.
|
|
|
|
The obvious downside of 2. is that everybody will have to "suffer" the
|
|
performance penalty incurred from calling `is_ntfs_dotgit()` on every
|
|
path, even in setups where.
|
|
|
|
After the recent work to accelerate `is_ntfs_dotgit()` in most cases,
|
|
it looks as if the time spent on validating ten million random
|
|
file names increases only negligibly (less than 20ms, well within the
|
|
standard deviation of ~50ms). Therefore the benefits outweigh the cost.
|
|
|
|
Another downside of this is that paths that might have been acceptable
|
|
previously now will be forbidden. Realistically, though, this is an
|
|
improvement because public Git hosters already would reject any `git
|
|
push` that contains such file names.
|
|
|
|
Note: There might be a similar problem mounting HFS+ on Linux. However,
|
|
this scenario has been considered unlikely and in light of the cost (in
|
|
the aforementioned benchmark, `core.protectHFS = true` increased the
|
|
time from ~440ms to ~610ms), it was decided _not_ to touch the default
|
|
of `core.protectHFS`.
|
|
|
|
This change addresses CVE-2019-1353.
|
|
|
|
Reported-by: Nicolas Joly <Nicolas.Joly@microsoft.com>
|
|
Helped-by: Garima Singh <garima.singh@microsoft.com>
|
|
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
|
---
|
|
config.mak.uname | 3 +--
|
|
environment.c | 2 +-
|
|
2 files changed, 2 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/config.mak.uname b/config.mak.uname
|
|
index db7f06b..3e46a8e 100644
|
|
--- a/config.mak.uname
|
|
+++ b/config.mak.uname
|
|
@@ -454,7 +454,6 @@ ifneq ($(USE_MSVC_CRTDBG),)
|
|
# Optionally enable memory leak reporting.
|
|
BASIC_CFLAGS += -DUSE_MSVC_CRTDBG
|
|
endif
|
|
- BASIC_CFLAGS += -DPROTECT_NTFS_DEFAULT=1
|
|
# Always give "-Zi" to the compiler and "-debug" to linker (even in
|
|
# release mode) to force a PDB to be generated (like RelWithDebInfo).
|
|
BASIC_CFLAGS += -Zi
|
|
@@ -616,7 +615,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
|
|
compat/win32/path-utils.o \
|
|
compat/win32/pthread.o compat/win32/syslog.o \
|
|
compat/win32/dirent.o
|
|
- BASIC_CFLAGS += -DWIN32 -DPROTECT_NTFS_DEFAULT=1
|
|
+ BASIC_CFLAGS += -DWIN32
|
|
EXTLIBS += -lws2_32
|
|
GITLIBS += git.res
|
|
PTHREAD_LIBS =
|
|
diff --git a/environment.c b/environment.c
|
|
index 89af47c..9f5f381 100644
|
|
--- a/environment.c
|
|
+++ b/environment.c
|
|
@@ -80,7 +80,7 @@
|
|
int protect_hfs = PROTECT_HFS_DEFAULT;
|
|
|
|
#ifndef PROTECT_NTFS_DEFAULT
|
|
-#define PROTECT_NTFS_DEFAULT 0
|
|
+#define PROTECT_NTFS_DEFAULT 1
|
|
#endif
|
|
int protect_ntfs = PROTECT_NTFS_DEFAULT;
|
|
const char *core_fsmonitor;
|
|
--
|
|
1.8.3.1
|
|
|