| blob | bee15e40b2fbf36236229e736f13386bd8c5f90d |
1 From 78ae55f7809eca86d980920062f6dd5d332e667f Mon Sep 17 00:00:00 2001
2 From: Johannes Schindelin <johannes.schindelin@gmx.de>
3 Date: Mon, 23 Sep 2019 08:58:11 +0200
4 Subject: is_ntfs_dotgit(): only verify the leading segment
6 The config setting `core.protectNTFS` is specifically designed to work
7 not only on Windows, but anywhere, to allow for repositories hosted on,
8 say, Linux servers to be protected against NTFS-specific attack vectors.
10 As a consequence, `is_ntfs_dotgit()` manually splits backslash-separated
11 paths (but does not do the same for paths separated by forward slashes),
12 under the assumption that the backslash might not be a valid directory
13 separator on the _current_ Operating System.
15 However, the two callers, `verify_path()` and `fsck_tree()`, are
16 supposed to feed only individual path segments to the `is_ntfs_dotgit()`
17 function.
19 This causes a lot of duplicate scanning (and very inefficient scanning,
20 too, as the inner loop of `is_ntfs_dotgit()` was optimized for
21 readability rather than for speed.
23 Let's simplify the design of `is_ntfs_dotgit()` by putting the burden of
24 splitting the paths by backslashes as directory separators on the
25 callers of said function.
27 Consequently, the `verify_path()` function, which already splits the
28 path by directory separators, now treats backslashes as directory
29 separators _explicitly_ when `core.protectNTFS` is turned on, even on
30 platforms where the backslash is _not_ a directory separator.
32 Note that we have to repeat some code in `verify_path()`: if the
33 backslash is not a directory separator on the current Operating System,
34 we want to allow file names like `\`, but we _do_ want to disallow paths
35 that are clearly intended to cause harm when the repository is cloned on
36 Windows.
38 The `fsck_tree()` function (the other caller of `is_ntfs_dotgit()`) now
39 needs to look for backslashes in tree entries' names specifically when
40 `core.protectNTFS` is turned on. While it would be tempting to
41 completely disallow backslashes in that case (much like `fsck` reports
42 names containing forward slashes as "full paths"), this would be
43 overzealous: when `core.protectNTFS` is turned on in a non-Windows
44 setup, backslashes are perfectly valid characters in file names while we
45 _still_ want to disallow tree entries that are clearly designed to
46 exploit NTFS-specific behavior.
48 This simplification will make subsequent changes easier to implement,
49 such as turning `core.protectNTFS` on by default (not only on Windows)
50 or protecting against attack vectors involving NTFS Alternate Data
51 Streams.
53 Incidentally, this change allows for catching malicious repositories
54 that contain tree entries of the form `dir\.gitmodules` already on the
55 server side rather than only on the client side (and previously only on
56 Windows): in contrast to `is_ntfs_dotgit()`, the
57 `is_ntfs_dotgitmodules()` function already expects the caller to split
58 the paths by directory separators.
60 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
61 (cherry picked from commit 288a74bcd28229a00c3632f18cba92dbfdf73ee9)
62 Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
63 ---
64 fsck.c | 18 +++++++++++++++++-
65 path.c | 5 +----
66 read-cache.c | 8 ++++++++
67 3 files changed, 26 insertions(+), 5 deletions(-)
69 diff --git a/fsck.c b/fsck.c
70 index cdb7d8db03..4faeeedf5d 100644
71 --- a/fsck.c
72 +++ b/fsck.c
73 @@ -570,7 +570,7 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
75 while (desc.size) {
76 unsigned short mode;
77 - const char *name;
78 + const char *name, *backslash;
79 const struct object_id *oid;
81 oid = tree_entry_extract(&desc, &name, &mode);
82 @@ -592,6 +592,22 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
83 ".gitmodules is a symbolic link");
84 }
86 + if ((backslash = strchr(name, '\\'))) {
87 + while (backslash) {
88 + backslash++;
89 + has_dotgit |= is_ntfs_dotgit(backslash);
90 + if (is_ntfs_dotgitmodules(backslash)) {
91 + if (!S_ISLNK(mode))
92 + oidset_insert(&gitmodules_found, oid);
93 + else
94 + retval += report(options, &item->object,
95 + FSCK_MSG_GITMODULES_SYMLINK,
96 + ".gitmodules is a symbolic link");
97 + }
98 + backslash = strchr(backslash, '\\');
99 + }
100 + }
101 +
102 if (update_tree_entry_gently(&desc)) {
103 retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
104 break;
105 diff --git a/path.c b/path.c
106 index 1374a0e5f1..95d53f0067 100644
107 --- a/path.c
108 +++ b/path.c
109 @@ -1370,10 +1370,7 @@ int is_ntfs_dotgit(const char *name)
110 if (only_spaces_and_periods(name, len, 5) &&
111 !strncasecmp(name, "git~1", 5))
112 return 1;
113 - if (name[len] != '\\')
114 - return 0;
115 - name += len + 1;
116 - len = -1;
117 + return 0;
118 }
119 }
121 diff --git a/read-cache.c b/read-cache.c
122 index 133f790fa4..f6e3fbdb79 100644
123 --- a/read-cache.c
124 +++ b/read-cache.c
125 @@ -991,7 +991,15 @@ int verify_path(const char *path, unsigned mode)
126 if ((c == '.' && !verify_dotfile(path, mode)) ||
127 is_dir_sep(c) || c == '\0')
128 return 0;
129 + } else if (c == '\\' && protect_ntfs) {
130 + if (is_ntfs_dotgit(path))
131 + return 0;
132 + if (S_ISLNK(mode)) {
133 + if (is_ntfs_dotgitmodules(path))
134 + return 0;
135 + }
136 }
137 +
138 c = *path++;
139 }
140 }
141 --
142 2.24.0.393.g34dc348eaf