Skip to content

Commit

Permalink
mingw: handle subst-ed "DOS drives"
Browse files Browse the repository at this point in the history
Over a decade ago, in 25fe217 (Windows: Treat Windows style path
names., 2008-03-05), Git was taught to handle absolute Windows paths,
i.e. paths that start with a drive letter and a colon.

Unbeknownst to us, while drive letters of physical drives are limited to
letters of the English alphabet, there is a way to assign virtual drive
letters to arbitrary directories, via the `subst` command, which is
_not_ limited to English letters.

It is therefore possible to have absolute Windows paths of the form
`1:\what\the\hex.txt`. Even "better": pretty much arbitrary Unicode
letters can also be used, e.g. `ä:\tschibät.sch`.

While it can be sensibly argued that users who set up such funny drive
letters really seek adverse consequences, the Windows Operating System
is known to be a platform where many users are at the mercy of
administrators who have their very own idea of what constitutes a
reasonable setup.

Therefore, let's just make sure that such funny paths are still
considered absolute paths by Git, on Windows.

In addition to Unicode characters, pretty much any character is a valid
drive letter, as far as `subst` is concerned, even `:` and `"` or even a
space character. While it is probably the opposite of smart to use them,
let's safeguard `is_dos_drive_prefix()` against all of them.

Note: `[::1]:repo` is a valid URL, but not a valid path on Windows.
As `[` is now considered a valid drive letter, we need to be very
careful to avoid misinterpreting such a string as valid local path in
`url_is_local_not_ssh()`. To do that, we use the just-introduced
function `is_valid_path()` (which will label the string as invalid file
name because of the colon characters).

This fixes CVE-2019-1351.

Reported-by: Nicolas Joly <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
dscho committed Dec 5, 2019
1 parent 817ddd6 commit f82a97e
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 3 deletions.
24 changes: 24 additions & 0 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,30 @@ pid_t waitpid(pid_t pid, int *status, int options)
return -1;
}

int mingw_has_dos_drive_prefix(const char *path)
{
int i;

/*
* Does it start with an ASCII letter (i.e. highest bit not set),
* followed by a colon?
*/
if (!(0x80 & (unsigned char)*path))
return *path && path[1] == ':' ? 2 : 0;

/*
* While drive letters must be letters of the English alphabet, it is
* possible to assign virtually _any_ Unicode character via `subst` as
* a drive letter to "virtual drives". Even `1`, or `ä`. Or fun stuff
* like this:
*
* subst ֍: %USERPROFILE%\Desktop
*/
for (i = 1; i < 4 && (0x80 & (unsigned char)path[i]); i++)
; /* skip first UTF-8 character */
return path[i] == ':' ? i + 1 : 0;
}

int mingw_skip_dos_drive_prefix(char **path)
{
int ret = has_dos_drive_prefix(*path);
Expand Down
4 changes: 2 additions & 2 deletions compat/mingw.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,8 @@ HANDLE winansi_get_osfhandle(int fd);
* git specific compatibility
*/

#define has_dos_drive_prefix(path) \
(isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int mingw_has_dos_drive_prefix(const char *path);
#define has_dos_drive_prefix mingw_has_dos_drive_prefix
int mingw_skip_dos_drive_prefix(char **path);
#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
static inline int mingw_is_dir_sep(int c)
Expand Down
2 changes: 1 addition & 1 deletion connect.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ int url_is_local_not_ssh(const char *url)
const char *colon = strchr(url, ':');
const char *slash = strchr(url, '/');
return !colon || (slash && slash < colon) ||
has_dos_drive_prefix(url);
(has_dos_drive_prefix(url) && is_valid_path(url));
}

static const char *prot_name(enum protocol protocol)
Expand Down
9 changes: 9 additions & 0 deletions t/t0060-path-utils.sh
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ test_expect_success 'absolute path rejects the empty string' '
test_must_fail test-path-utils absolute_path ""
'

test_expect_success MINGW '<drive-letter>:\\abc is an absolute path' '
for letter in : \" C Z 1 ä
do
path=$letter:\\abc &&
absolute="$(test-path-utils absolute_path "$path")" &&
test "$path" = "$absolute" || return 1
done
'

test_expect_success 'real path rejects the empty string' '
test_must_fail test-path-utils real_path ""
'
Expand Down

0 comments on commit f82a97e

Please sign in to comment.