summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohannes Schindelin <johannes.schindelin@gmx.de>2019-06-19 12:59:27 +0200
committerEdward Thomson <ethomson@edwardthomson.com>2019-08-05 00:03:50 +0100
commitd475d5d66029084f4229a7c6da2505cfbcaf93e3 (patch)
tree765aac2593a68f3d9ffc094f5376b99bd46aa22b
parentb3e1a56ebb2b9291e82dc027ba9cbcfc3ead54d3 (diff)
downloadlibgit2-d475d5d66029084f4229a7c6da2505cfbcaf93e3.tar.gz
config: validate ownership of C:\ProgramData\Git\config before using it
When the VirtualStore feature is in effect, it is safe to let random users write into C:\ProgramData because other users won't see those files. This seemed to be the case when we introduced support for C:\ProgramData\Git\config. However, when that feature is not in effect (which seems to be the case in newer Windows 10 versions), we'd rather not use those files unless they come from a trusted source, such as an administrator. This change imitates the strategy chosen by PowerShell's native OpenSSH port to Windows regarding host key files: if a system file is owned neither by an administrator, a system account, or the current user, it is ignored.
-rw-r--r--src/config.c9
-rw-r--r--src/path.c77
-rw-r--r--src/path.h12
3 files changed, 97 insertions, 1 deletions
diff --git a/src/config.c b/src/config.c
index fce1aad3b..fed0bfb29 100644
--- a/src/config.c
+++ b/src/config.c
@@ -1111,8 +1111,15 @@ int git_config_find_system(git_buf *path)
int git_config_find_programdata(git_buf *path)
{
+ int ret;
+
git_buf_sanitize(path);
- return git_sysdir_find_programdata_file(path, GIT_CONFIG_FILENAME_PROGRAMDATA);
+ ret = git_sysdir_find_programdata_file(path,
+ GIT_CONFIG_FILENAME_PROGRAMDATA);
+ if (ret != GIT_OK)
+ return ret;
+
+ return git_path_validate_system_file_ownership(path->ptr);
}
int git_config__global_location(git_buf *buf)
diff --git a/src/path.c b/src/path.c
index 197efb75a..90473fc19 100644
--- a/src/path.c
+++ b/src/path.c
@@ -14,6 +14,7 @@
#include "win32/w32_buffer.h"
#include "win32/w32_util.h"
#include "win32/version.h"
+#include <AclAPI.h>
#else
#include <dirent.h>
#endif
@@ -1909,3 +1910,79 @@ extern int git_path_is_gitfile(const char *path, size_t pathlen, git_path_gitfil
return -1;
}
}
+
+int git_path_validate_system_file_ownership(const char *path)
+{
+#ifndef GIT_WIN32
+ GIT_UNUSED(path);
+ return GIT_OK;
+#else
+ git_win32_path buf;
+ PSID owner_sid;
+ PSECURITY_DESCRIPTOR descriptor = NULL;
+ HANDLE token;
+ TOKEN_USER *info = NULL;
+ DWORD err, len;
+ int ret;
+
+ if (git_win32_path_from_utf8(buf, path) < 0)
+ return -1;
+
+ err = GetNamedSecurityInfoW(buf, SE_FILE_OBJECT,
+ OWNER_SECURITY_INFORMATION |
+ DACL_SECURITY_INFORMATION,
+ &owner_sid, NULL, NULL, NULL, &descriptor);
+
+ if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
+ ret = GIT_ENOTFOUND;
+ goto cleanup;
+ }
+
+ if (err != ERROR_SUCCESS) {
+ git_error_set(GIT_ERROR_OS, "failed to get security information");
+ ret = GIT_ERROR;
+ goto cleanup;
+ }
+
+ if (!IsValidSid(owner_sid)) {
+ git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is unknown");
+ ret = GIT_ERROR;
+ goto cleanup;
+ }
+
+ if (IsWellKnownSid(owner_sid, WinBuiltinAdministratorsSid) ||
+ IsWellKnownSid(owner_sid, WinLocalSystemSid)) {
+ ret = GIT_OK;
+ goto cleanup;
+ }
+
+ /* Obtain current user's SID */
+ if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token) &&
+ !GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
+ info = git__malloc(len);
+ GIT_ERROR_CHECK_ALLOC(info);
+ if (!GetTokenInformation(token, TokenUser, info, len, &len)) {
+ git__free(info);
+ info = NULL;
+ }
+ }
+
+ /*
+ * If the file is owned by the same account that is running the current
+ * process, it's okay to read from that file.
+ */
+ if (info && EqualSid(owner_sid, info->User.Sid))
+ ret = GIT_OK;
+ else {
+ git_error_set(GIT_ERROR_INVALID, "programdata configuration file owner is not valid");
+ ret = GIT_ERROR;
+ }
+ free(info);
+
+cleanup:
+ if (descriptor)
+ LocalFree(descriptor);
+
+ return ret;
+#endif
+}
diff --git a/src/path.h b/src/path.h
index e29a7f6b3..5fac695e9 100644
--- a/src/path.h
+++ b/src/path.h
@@ -647,4 +647,16 @@ extern bool git_path_isvalid(
*/
int git_path_normalize_slashes(git_buf *out, const char *path);
+/**
+ * Validate a system file's ownership
+ *
+ * Verify that the file in question is owned by an administrator or system
+ * account, or at least by the current user.
+ *
+ * This function returns 0 if successful. If the file is not owned by any of
+ * these, or any other if there have been problems determining the file
+ * ownership, it returns -1.
+ */
+int git_path_validate_system_file_ownership(const char *path);
+
#endif