Skip to content

Commit 2ddcccf

Browse files
committed
Merge branch 'win32-accommodate-funny-drive-names'
While the only permitted drive letters for physical drives on Windows are letters of the US-English alphabet, this restriction does not apply to virtual drives assigned via `subst <letter>: <path>`. To prevent targeted attacks against systems where "funny" drive letters such as `1` or `!` are assigned, let's handle them as regular drive letters on Windows. This fixes CVE-2019-1351. Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 65d30a1 + f82a97e commit 2ddcccf

File tree

4 files changed

+54
-6
lines changed

4 files changed

+54
-6
lines changed

compat/mingw.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,6 +1989,30 @@ pid_t waitpid(pid_t pid, int *status, int options)
19891989
return -1;
19901990
}
19911991

1992+
int mingw_has_dos_drive_prefix(const char *path)
1993+
{
1994+
int i;
1995+
1996+
/*
1997+
* Does it start with an ASCII letter (i.e. highest bit not set),
1998+
* followed by a colon?
1999+
*/
2000+
if (!(0x80 & (unsigned char)*path))
2001+
return *path && path[1] == ':' ? 2 : 0;
2002+
2003+
/*
2004+
* While drive letters must be letters of the English alphabet, it is
2005+
* possible to assign virtually _any_ Unicode character via `subst` as
2006+
* a drive letter to "virtual drives". Even `1`, or `ä`. Or fun stuff
2007+
* like this:
2008+
*
2009+
* subst ֍: %USERPROFILE%\Desktop
2010+
*/
2011+
for (i = 1; i < 4 && (0x80 & (unsigned char)path[i]); i++)
2012+
; /* skip first UTF-8 character */
2013+
return path[i] == ':' ? i + 1 : 0;
2014+
}
2015+
19922016
int mingw_skip_dos_drive_prefix(char **path)
19932017
{
19942018
int ret = has_dos_drive_prefix(*path);
@@ -2137,6 +2161,8 @@ int is_valid_win32_path(const char *path)
21372161
if (!protect_ntfs)
21382162
return 1;
21392163

2164+
skip_dos_drive_prefix((char **)&path);
2165+
21402166
for (;;) {
21412167
char c = *(path++);
21422168
switch (c) {
@@ -2158,6 +2184,14 @@ int is_valid_win32_path(const char *path)
21582184
preceding_space_or_period = 1;
21592185
i++;
21602186
continue;
2187+
case ':': /* DOS drive prefix was already skipped */
2188+
case '<': case '>': case '"': case '|': case '?': case '*':
2189+
/* illegal character */
2190+
return 0;
2191+
default:
2192+
if (c > '\0' && c < '\x20')
2193+
/* illegal character */
2194+
return 0;
21612195
}
21622196
preceding_space_or_period = 0;
21632197
i++;

compat/mingw.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,8 @@ HANDLE winansi_get_osfhandle(int fd);
394394
* git specific compatibility
395395
*/
396396

397-
#define has_dos_drive_prefix(path) \
398-
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
397+
int mingw_has_dos_drive_prefix(const char *path);
398+
#define has_dos_drive_prefix mingw_has_dos_drive_prefix
399399
int mingw_skip_dos_drive_prefix(char **path);
400400
#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
401401
static inline int mingw_is_dir_sep(int c)
@@ -431,8 +431,11 @@ int mingw_offset_1st_component(const char *path);
431431
/**
432432
* Verifies that the given path is a valid one on Windows.
433433
*
434-
* In particular, path segments are disallowed which end in a period or a
435-
* space (except the special directories `.` and `..`).
434+
* In particular, path segments are disallowed which
435+
*
436+
* - end in a period or a space (except the special directories `.` and `..`).
437+
*
438+
* - contain any of the reserved characters, e.g. `:`, `;`, `*`, etc
436439
*
437440
* Returns 1 upon success, otherwise 0.
438441
*/

connect.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ int url_is_local_not_ssh(const char *url)
264264
const char *colon = strchr(url, ':');
265265
const char *slash = strchr(url, '/');
266266
return !colon || (slash && slash < colon) ||
267-
has_dos_drive_prefix(url);
267+
(has_dos_drive_prefix(url) && is_valid_path(url));
268268
}
269269

270270
static const char *prot_name(enum protocol protocol)

t/t0060-path-utils.sh

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,15 @@ test_expect_success 'absolute path rejects the empty string' '
165165
test_must_fail test-path-utils absolute_path ""
166166
'
167167

168+
test_expect_success MINGW '<drive-letter>:\\abc is an absolute path' '
169+
for letter in : \" C Z 1 ä
170+
do
171+
path=$letter:\\abc &&
172+
absolute="$(test-path-utils absolute_path "$path")" &&
173+
test "$path" = "$absolute" || return 1
174+
done
175+
'
176+
168177
test_expect_success 'real path rejects the empty string' '
169178
test_must_fail test-path-utils real_path ""
170179
'
@@ -445,13 +454,15 @@ test_expect_success MINGW 'is_valid_path() on Windows' '
445454
win32 \
446455
"win32 x" \
447456
../hello.txt \
457+
C:\\git \
448458
\
449459
--not \
450460
"win32 " \
451461
"win32 /x " \
452462
"win32." \
453463
"win32 . ." \
454-
.../hello.txt
464+
.../hello.txt \
465+
colon:test
455466
'
456467

457468
test_done

0 commit comments

Comments
 (0)