Merge pull request #5202 from libgit2/users/ethomson/security_updates

Security updates from 0.28.3

5 files changed, 113 insertions, 3 deletions

diff --git a/docs/changelog.md b/docs/changelog.md
index e5eaf0794..563c5c9c8 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 44673d9bc..78b1f055c 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 862476719..d0e439b55 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 41232c2f6..150e09eb6 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 624ca03aa..ed6b93574 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