Merge pull request #5202 from libgit2/users/ethomson/security_updates Security updates from 0.28.3
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 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177
diff --git a/docs/changelog.md b/docs/changelog.md
index e5eaf07..563c5c9 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -22,6 +22,16 @@ v0.28 + 1
* libgit2 can now correctly cope with URLs where the host contains a colon
but a port is not specified. (eg `http://example.com:/repo.git`).
+* A carefully constructed commit object with a very large number
+ of parents may lead to potential out-of-bounds writes or
+ potential denial of service.
+
+* The ProgramData configuration file is always read for compatibility
+ with Git for Windows and Portable Git installations. The ProgramData
+ location is not necessarily writable only by administrators, so we
+ now ensure that the configuration file is owned by the administrator
+ or the current user.
+
v0.28
-----
diff --git a/src/commit_list.c b/src/commit_list.c
index 44673d9..78b1f05 100644
--- a/src/commit_list.c
+++ b/src/commit_list.c
@@ -69,11 +69,15 @@ static int commit_error(git_commit_list_node *commit, const char *msg)
static git_commit_list_node **alloc_parents(
git_revwalk *walk, git_commit_list_node *commit, size_t n_parents)
{
+ size_t bytes;
+
if (n_parents <= PARENTS_PER_COMMIT)
return (git_commit_list_node **)((char *)commit + sizeof(git_commit_list_node));
- return (git_commit_list_node **)git_pool_malloc(
- &walk->commit_pool, (n_parents * sizeof(git_commit_list_node *)));
+ if (git__multiply_sizet_overflow(&bytes, n_parents, sizeof(git_commit_list_node *)))
+ return NULL;
+
+ return (git_commit_list_node **)git_pool_malloc(&walk->commit_pool, bytes);
}
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