Skip to content

Commit 7f3551d

Browse files
committed
Merge branch 'disallow-dotgit-via-ntfs-alternate-data-streams'
This patch series plugs an attack vector we had overlooked in our December 2014 work on `core.protectNTFS`. Essentially, the path `.git::$INDEX_ALLOCATION/config` is interpreted as `.git/config` when NTFS Alternate Data Streams are available (which they are on Windows, and at least on network shares that are SMB-mounted on macOS). Needless to say: we don't want that. In fact, we want to stay on the very safe side and not even special-case the `$INDEX_ALLOCATION` stream type: let's just prevent Git from touching _any_ explicitly specified Alternate Data Stream of `.git`. In essence, we'll prevent Git from tracking, or writing to, any path with a segment of the form `.git:<anything>`. Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 4778452 + 91bd465 commit 7f3551d

File tree

6 files changed

+162
-8
lines changed

6 files changed

+162
-8
lines changed

fsck.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
551551

552552
while (desc.size) {
553553
unsigned mode;
554-
const char *name;
554+
const char *name, *backslash;
555555
const struct object_id *oid;
556556

557557
oid = tree_entry_extract(&desc, &name, &mode);
@@ -565,6 +565,15 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
565565
is_hfs_dotgit(name) ||
566566
is_ntfs_dotgit(name));
567567
has_zero_pad |= *(char *)desc.buffer == '0';
568+
569+
if ((backslash = strchr(name, '\\'))) {
570+
while (backslash) {
571+
backslash++;
572+
has_dotgit |= is_ntfs_dotgit(backslash);
573+
backslash = strchr(backslash, '\\');
574+
}
575+
}
576+
568577
if (update_tree_entry_gently(&desc)) {
569578
retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
570579
break;

path.c

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,22 +1302,57 @@ static int only_spaces_and_periods(const char *path, size_t len, size_t skip)
13021302
return 1;
13031303
}
13041304

1305+
/*
1306+
* On NTFS, we need to be careful to disallow certain synonyms of the `.git/`
1307+
* directory:
1308+
*
1309+
* - For historical reasons, file names that end in spaces or periods are
1310+
* automatically trimmed. Therefore, `.git . . ./` is a valid way to refer
1311+
* to `.git/`.
1312+
*
1313+
* - For other historical reasons, file names that do not conform to the 8.3
1314+
* format (up to eight characters for the basename, three for the file
1315+
* extension, certain characters not allowed such as `+`, etc) are associated
1316+
* with a so-called "short name", at least on the `C:` drive by default.
1317+
* Which means that `git~1/` is a valid way to refer to `.git/`.
1318+
*
1319+
* Note: Technically, `.git/` could receive the short name `git~2` if the
1320+
* short name `git~1` were already used. In Git, however, we guarantee that
1321+
* `.git` is the first item in a directory, therefore it will be associated
1322+
* with the short name `git~1` (unless short names are disabled).
1323+
*
1324+
* - For yet other historical reasons, NTFS supports so-called "Alternate Data
1325+
* Streams", i.e. metadata associated with a given file, referred to via
1326+
* `<filename>:<stream-name>:<stream-type>`. There exists a default stream
1327+
* type for directories, allowing `.git/` to be accessed via
1328+
* `.git::$INDEX_ALLOCATION/`.
1329+
*
1330+
* When this function returns 1, it indicates that the specified file/directory
1331+
* name refers to a `.git` file or directory, or to any of these synonyms, and
1332+
* Git should therefore not track it.
1333+
*
1334+
* For performance reasons, _all_ Alternate Data Streams of `.git/` are
1335+
* forbidden, not just `::$INDEX_ALLOCATION`.
1336+
*
1337+
* This function is intended to be used by `git fsck` even on platforms where
1338+
* the backslash is a regular filename character, therefore it needs to handle
1339+
* backlash characters in the provided `name` specially: they are interpreted
1340+
* as directory separators.
1341+
*/
13051342
int is_ntfs_dotgit(const char *name)
13061343
{
13071344
size_t len;
13081345

13091346
for (len = 0; ; len++)
1310-
if (!name[len] || name[len] == '\\' || is_dir_sep(name[len])) {
1347+
if (!name[len] || name[len] == '\\' || is_dir_sep(name[len]) ||
1348+
name[len] == ':') {
13111349
if (only_spaces_and_periods(name, len, 4) &&
13121350
!strncasecmp(name, ".git", 4))
13131351
return 1;
13141352
if (only_spaces_and_periods(name, len, 5) &&
13151353
!strncasecmp(name, "git~1", 5))
13161354
return 1;
1317-
if (name[len] != '\\')
1318-
return 0;
1319-
name += len + 1;
1320-
len = -1;
1355+
return 0;
13211356
}
13221357
}
13231358

@@ -1334,7 +1369,7 @@ static int is_ntfs_dot_generic(const char *name,
13341369
only_spaces_and_periods:
13351370
for (;;) {
13361371
char c = name[i++];
1337-
if (!c)
1372+
if (!c || c == ':')
13381373
return 1;
13391374
if (c != ' ' && c != '.')
13401375
return 0;

read-cache.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,7 +874,15 @@ int verify_path(const char *path, unsigned mode)
874874
if ((c == '.' && !verify_dotfile(path, mode)) ||
875875
is_dir_sep(c) || c == '\0')
876876
return 0;
877+
} else if (c == '\\' && protect_ntfs) {
878+
if (is_ntfs_dotgit(path))
879+
return 0;
880+
if (S_ISLNK(mode)) {
881+
if (is_ntfs_dotgitmodules(path))
882+
return 0;
883+
}
877884
}
885+
878886
c = *path++;
879887
}
880888
}

t/helper/test-path-utils.c

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,99 @@ static int is_dotgitmodules(const char *path)
176176
return is_hfs_dotgitmodules(path) || is_ntfs_dotgitmodules(path);
177177
}
178178

179+
/*
180+
* A very simple, reproducible pseudo-random generator. Copied from
181+
* `test-genrandom.c`.
182+
*/
183+
static uint64_t my_random_value = 1234;
184+
185+
static uint64_t my_random(void)
186+
{
187+
my_random_value = my_random_value * 1103515245 + 12345;
188+
return my_random_value;
189+
}
190+
191+
/*
192+
* A fast approximation of the square root, without requiring math.h.
193+
*
194+
* It uses Newton's method to approximate the solution of 0 = x^2 - value.
195+
*/
196+
static double my_sqrt(double value)
197+
{
198+
const double epsilon = 1e-6;
199+
double x = value;
200+
201+
if (value == 0)
202+
return 0;
203+
204+
for (;;) {
205+
double delta = (value / x - x) / 2;
206+
if (delta < epsilon && delta > -epsilon)
207+
return x + delta;
208+
x += delta;
209+
}
210+
}
211+
212+
static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
213+
{
214+
size_t i, j, nr, min_len = 3, max_len = 20;
215+
char **names;
216+
int repetitions = 15, file_mode = 0100644;
217+
uint64_t begin, end;
218+
double m[3][2], v[3][2];
219+
uint64_t cumul;
220+
double cumul2;
221+
222+
if (argc > 1 && !strcmp(argv[1], "--with-symlink-mode")) {
223+
file_mode = 0120000;
224+
argc--;
225+
argv++;
226+
}
227+
228+
nr = argc > 1 ? strtoul(argv[1], NULL, 0) : 1000000;
229+
ALLOC_ARRAY(names, nr);
230+
231+
if (argc > 2) {
232+
min_len = strtoul(argv[2], NULL, 0);
233+
if (argc > 3)
234+
max_len = strtoul(argv[3], NULL, 0);
235+
if (min_len > max_len)
236+
die("min_len > max_len");
237+
}
238+
239+
for (i = 0; i < nr; i++) {
240+
size_t len = min_len + (my_random() % (max_len + 1 - min_len));
241+
242+
names[i] = xmallocz(len);
243+
while (len > 0)
244+
names[i][--len] = (char)(' ' + (my_random() % ('\x7f' - ' ')));
245+
}
246+
247+
for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
248+
for (protect_hfs = 0; protect_hfs < 2; protect_hfs++) {
249+
cumul = 0;
250+
cumul2 = 0;
251+
for (i = 0; i < repetitions; i++) {
252+
begin = getnanotime();
253+
for (j = 0; j < nr; j++)
254+
verify_path(names[j], file_mode);
255+
end = getnanotime();
256+
printf("protect_ntfs = %d, protect_hfs = %d: %lfms\n", protect_ntfs, protect_hfs, (end-begin) / (double)1e6);
257+
cumul += end - begin;
258+
cumul2 += (end - begin) * (end - begin);
259+
}
260+
m[protect_ntfs][protect_hfs] = cumul / (double)repetitions;
261+
v[protect_ntfs][protect_hfs] = my_sqrt(cumul2 / (double)repetitions - m[protect_ntfs][protect_hfs] * m[protect_ntfs][protect_hfs]);
262+
printf("mean: %lfms, stddev: %lfms\n", m[protect_ntfs][protect_hfs] / (double)1e6, v[protect_ntfs][protect_hfs] / (double)1e6);
263+
}
264+
265+
for (protect_ntfs = 0; protect_ntfs < 2; protect_ntfs++)
266+
for (protect_hfs = 0; protect_hfs < 2; protect_hfs++)
267+
printf("ntfs=%d/hfs=%d: %lf%% slower\n", protect_ntfs, protect_hfs, (m[protect_ntfs][protect_hfs] - m[0][0]) * 100 / m[0][0]);
268+
269+
return 0;
270+
}
271+
179272
int cmd_main(int argc, const char **argv)
180273
{
181274
if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
@@ -290,6 +383,9 @@ int cmd_main(int argc, const char **argv)
290383
return !!res;
291384
}
292385

386+
if (argc > 1 && !strcmp(argv[1], "protect_ntfs_hfs"))
387+
return !!protect_ntfs_hfs_benchmark(argc - 1, argv + 1);
388+
293389
fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
294390
argv[1] ? argv[1] : "(there was none)");
295391
return 1;

t/t0060-path-utils.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,9 @@ test_expect_success 'match .gitmodules' '
408408
~1000000 \
409409
~9999999 \
410410
\
411+
.gitmodules:\$DATA \
412+
"gitmod~4 . :\$DATA" \
413+
\
411414
--not \
412415
".gitmodules x" \
413416
".gitmodules .x" \
@@ -432,7 +435,9 @@ test_expect_success 'match .gitmodules' '
432435
\
433436
GI7EB~1 \
434437
GI7EB~01 \
435-
GI7EB~1X
438+
GI7EB~1X \
439+
\
440+
.gitmodules,:\$DATA
436441
'
437442

438443
test_done

t/t1014-read-tree-confusing.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ git~1
4949
.git.SPACE .git.{space}
5050
.\\\\.GIT\\\\foobar backslashes
5151
.git\\\\foobar backslashes2
52+
.git...:alternate-stream
5253
EOF
5354

5455
test_expect_success 'utf-8 paths allowed with core.protectHFS off' '

0 commit comments

Comments
 (0)