Commit d475d5d66029084f4229a7c6da2505cfbcaf93e3
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 fce1aad..fed0bfb 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 197efb7..90473fc 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 e29a7f6..5fac695 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