Commit cb1439c9d32c059ee93216637a6d155306f76ab3
2019-06-19T12:59:27
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134
diff --git a/src/config.c b/src/config.c
index 8624767..d0e439b 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 41232c2..150e09e 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
@@ -1946,3 +1947,79 @@ done:
git_buf_dispose(&path);
return supported;
}
+
+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 624ca03..ed6b935 100644
--- a/src/path.h
+++ b/src/path.h
@@ -649,4 +649,16 @@ int git_path_normalize_slashes(git_buf *out, const char *path);
bool git_path_supports_symlinks(const char *dir);
+/**
+ * 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