Skip to content

Commit 8072346

Browse files
committed
mingw: safeguard better against backslashes in file names
In 224c7d7 (mingw: only test index entries for backslashes, not tree entries, 2019-12-31), we relaxed the check for backslashes in tree entries to check only index entries. However, the code change was incorrect: it was added to `add_index_entry_with_check()`, not to `add_index_entry()`, so under certain circumstances it was possible to side-step the protection. Besides, the description of that commit purported that all index entries would be checked when in fact they were only checked when being added to the index (there are code paths that do not do that, constructing "transient" index entries). In any case, it was pointed out in one insightful review at #2437 (comment) that it would be a much better idea to teach `verify_path()` to perform the check for a backslash. This is safer, even if it comes with two notable drawbacks: - `verify_path()` cannot say _what_ is wrong with the path, therefore the user will no longer be told that there was a backslash in the path, only that the path was invalid. - The `git apply` command also calls the `verify_path()` function, and might have been able to handle Windows-style paths (i.e. with backslashes instead of forward slashes). This will no longer be possible unless the user (temporarily) sets `core.protectNTFS=false`. Note that `git add <windows-path>` will _still_ work because `normalize_path_copy_len()` will convert the backslashes to forward slashes before hitting the code path that creates an index entry. The clear advantage is that `verify_path()`'s purpose is to check the validity of the file name, therefore we naturally tap into all the code paths that need safeguarding, also implicitly into future code paths. The benefits of that approach outweigh the downsides, so let's move the check from `add_index_entry_with_check()` to `verify_path()`. Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 39858a6 commit 8072346

File tree

2 files changed

+7
-7
lines changed

2 files changed

+7
-7
lines changed

read-cache.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ static int verify_dotfile(const char *rest, unsigned mode)
959959

960960
int verify_path(const char *path, unsigned mode)
961961
{
962-
char c;
962+
char c = 0;
963963

964964
if (has_dos_drive_prefix(path))
965965
return 0;
@@ -974,6 +974,7 @@ int verify_path(const char *path, unsigned mode)
974974
if (is_dir_sep(c)) {
975975
inside:
976976
if (protect_hfs) {
977+
977978
if (is_hfs_dotgit(path))
978979
return 0;
979980
if (S_ISLNK(mode)) {
@@ -982,6 +983,10 @@ int verify_path(const char *path, unsigned mode)
982983
}
983984
}
984985
if (protect_ntfs) {
986+
#ifdef GIT_WINDOWS_NATIVE
987+
if (c == '\\')
988+
return 0;
989+
#endif
985990
if (is_ntfs_dotgit(path))
986991
return 0;
987992
if (S_ISLNK(mode)) {
@@ -1278,11 +1283,6 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
12781283
int skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;
12791284
int new_only = option & ADD_CACHE_NEW_ONLY;
12801285

1281-
#ifdef GIT_WINDOWS_NATIVE
1282-
if (protect_ntfs && strchr(ce->name, '\\'))
1283-
return error(_("filename in tree entry contains backslash: '%s'"), ce->name);
1284-
#endif
1285-
12861286
if (!(option & ADD_CACHE_KEEP_CACHE_TREE))
12871287
cache_tree_invalidate_path(istate, ce->name);
12881288

t/t7415-submodule-names.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ test_expect_success MINGW 'prevent git~1 squatting on Windows' '
209209
hash="$(echo x | git hash-object -w --stdin)" &&
210210
test_must_fail git update-index --add \
211211
--cacheinfo 160000,$rev,d\\a 2>err &&
212-
test_i18ngrep backslash err &&
212+
test_i18ngrep "Invalid path" err &&
213213
git -c core.protectNTFS=false update-index --add \
214214
--cacheinfo 100644,$modules,.gitmodules \
215215
--cacheinfo 160000,$rev,c \

0 commit comments

Comments
 (0)