From fdea8cabf2e68f0ecaaca481974b579a569bcdee Mon Sep 17 00:00:00 2001 From: Sven Strickroth Date: Thu, 28 May 2026 19:04:50 +0200 Subject: [PATCH] Refuse to follow invalid paths in `.git` files This corresponds to the fixes in TortoiseGit v2.19 that were announced in https://groups.google.com/g/tortoisegit-announce/c/31zdmOBi4vY. The threat surface in Git for Windows is a lot smaller than in TortoiseGit because Git is not integrated directly into the Windows Explorer and hence does not run automatically on any extracted archive. A Git user would have to run Git operations on an extracted archive of dubious origin, which is never a good idea. In all other circumstances, the `.git` files Git would see would always be written by Git for Windows itself, which will never write `.git` files containing invalid paths (or paths pointing to other servers). For this reason, it was deemed okay to integrate this change as a regular patch. Signed-off-by: Sven Strickroth Signed-off-by: Johannes Schindelin --- compat/mingw.c | 33 +++++++++++++++++++++++++++++++++ compat/mingw.h | 1 + setup.c | 36 ++++++++++++++++++++++++++++++++++++ t/t5580-unc-paths.sh | 2 ++ 4 files changed, 72 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index fcbb04dc01..e513c52ed5 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -3299,6 +3299,30 @@ static int acls_supported(const char *path) return 0; } +int is_valid_windows_path_element(wchar_t ch) +{ + // cf. https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file + // Windows disallows ASCII control characters 0x00–0x1F + if (ch < 0x1F) + return false; + + // Windows reserved path characters + switch (ch) { + case L'<': + case L'>': + case L':': + case L'"': + case L'/': + case L'\\': + case L'|': + case L'?': + case L'*': + return 0; + default: + return 1; + } +} + int is_path_owned_by_current_sid(const char *path, struct strbuf *report) { WCHAR wpath[MAX_PATH]; @@ -3327,6 +3351,15 @@ int is_path_owned_by_current_sid(const char *path, struct strbuf *report) if (!wcsicmp(wpath, home)) return 1; + /* Do not leak NTLM hashes, UNC paths etc. are generally problematic */ + if (wpath[0] == L'\\' && !is_valid_windows_path_element(wpath[1])) { + if (report) + strbuf_addf(report, + "'%s' may refer to a non-local directory", + path); + return 0; + } + /* Get the owner SID */ err = GetNamedSecurityInfoW(wpath, SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION | diff --git a/compat/mingw.h b/compat/mingw.h index 444daedfa5..16345ffb19 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -46,6 +46,7 @@ char *mingw_query_user_email(void); */ int is_path_owned_by_current_sid(const char *path, struct strbuf *report); #define is_path_owned_by_current_user is_path_owned_by_current_sid +int is_valid_windows_path_element(wchar_t ch); /** * Verifies that the given path is a valid one on Windows. diff --git a/setup.c b/setup.c index b4652651df..e47cfb53e4 100644 --- a/setup.c +++ b/setup.c @@ -943,6 +943,35 @@ void read_gitfile_error_die(int error_code, const char *path, const char *dir) } } +#if (defined _WIN32 || defined __WIN32__) +static int ensure_valid_ownership(const char *gitfile, + const char *worktree, const char *gitdir, + struct strbuf *report); + +static int is_invalid_dotgit_path(const char *gitfile, const char *potential_gitdir) +{ + WCHAR wpath[MAX_PATH]; + struct strbuf dir = STRBUF_INIT; + int ret; + + if (xutftowcs_path(wpath, potential_gitdir) < 0) + return 1; + + /* Do not leak NTLM hashes, UNC paths etc. are generally problematic */ + if (wpath[0] != L'\\' || is_valid_windows_path_element(wpath[1])) + return 0; + + strbuf_addstr(&dir, gitfile); + strbuf_strip_file_from_path(&dir); + + ret = ensure_valid_ownership(gitfile, dir.buf, potential_gitdir, NULL) ? 0 : 1; + + strbuf_release(&dir); + + return ret; +} +#endif + /* * Try to read the location of the git directory from the .git file, * return path to git directory if found. The return value comes from @@ -1016,6 +1045,13 @@ const char *read_gitfile_gently(const char *path, int *return_error_code) free(buf); buf = dir; } +#if (defined _WIN32 || defined __WIN32__) + if (is_dir_sep(dir[0]) && is_invalid_dotgit_path(path, dir)) { + strbuf_add(&realpath, dir, strlen(dir)); + path = realpath.buf; + goto cleanup_return; + } +#endif if (!is_git_directory(dir)) { error_code = READ_GITFILE_ERR_NOT_A_REPO; goto cleanup_return; diff --git a/t/t5580-unc-paths.sh b/t/t5580-unc-paths.sh index 65ef1a3628..a2c7f9853a 100755 --- a/t/t5580-unc-paths.sh +++ b/t/t5580-unc-paths.sh @@ -44,6 +44,8 @@ test_expect_success clone ' ' test_expect_success 'clone without file://' ' + test_must_fail git clone "$UNCPATH" clone-without-file && + git config set --global --append safe.directory "$UNCPATH/.git" && git clone "$UNCPATH" clone-without-file '