From 2e4ea7b7fb593dea0b96e3bca659c3594e7fe734 Mon Sep 17 00:00:00 2001
From: Edward Thomson <ethomson@edwardthomson.com>
Date: Sat, 2 Jul 2022 10:19:33 -0400
Subject: [PATCH 19/20] repo: validate gitdir and gitlink ownership

To match git's behavior with CVE 2022-29187, validate not only the
working directory, but also the gitdir and gitlink (if it exists). This
a follow up to CVE-2022-24765 that was fixed earlier.
---
 src/repository.c | 107 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 30 deletions(-)

diff --git a/src/repository.c b/src/repository.c
index e770aa961..ac32085da 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -487,7 +487,7 @@ static int read_gitfile(git_buf *path_out, const char *file_path)
 typedef struct {
 	const char *repo_path;
 	git_buf tmp;
-	bool is_safe;
+	bool *is_safe;
 } validate_ownership_data;
 
 static int validate_ownership_cb(const git_config_entry *entry, void *payload)
@@ -495,52 +495,101 @@ static int validate_ownership_cb(const git_config_entry *entry, void *payload)
 	validate_ownership_data *data = payload;
 
 	if (strcmp(entry->value, "") == 0)
-		data->is_safe = false;
+		*data->is_safe = false;
 
 	if (git_path_prettify_dir(&data->tmp, entry->value, NULL) == 0 &&
 	    strcmp(data->tmp.ptr, data->repo_path) == 0)
-		data->is_safe = true;
+		*data->is_safe = true;
 
 	return 0;
 }
 
-static int validate_ownership(const char *repo_path)
+static int validate_ownership_config(bool *is_safe, const char *path)
+{
+	validate_ownership_data ownership_data = {
+		path, GIT_BUF_INIT, is_safe
+	};
+	git_config *config;
+	int error;
+
+	if (load_global_config(&config) != 0)
+		return 0;
+
+	error = git_config_get_multivar_foreach(config,
+		"safe.directory", NULL,
+		validate_ownership_cb,
+		&ownership_data);
+
+	git_config_free(config);
+	git_buf_dispose(&ownership_data.tmp);
+
+	return error;
+}
+
+static int validate_ownership_path(bool *is_safe, const char *path)
 {
-	git_config *config = NULL;
-	validate_ownership_data data = { repo_path, GIT_BUF_INIT, false };
 	git_path_owner_t owner_level =
 		GIT_PATH_OWNER_CURRENT_USER |
 		GIT_PATH_USER_IS_ADMINISTRATOR;
-	bool is_safe;
-	int error;
-
-	if ((error = git_path_owner_is(&is_safe, repo_path, owner_level)) < 0) {
-		if (error == GIT_ENOTFOUND)
-			error = 0;
+	int error = 0;
 
-		goto done;
-	}
+	if (path)
+		error = git_path_owner_is(is_safe, path, owner_level);
 
-	if (is_safe) {
+	if (error == GIT_ENOTFOUND) {
+		*is_safe = true;
 		error = 0;
-		goto done;
 	}
 
-	if (load_global_config(&config) == 0) {
-		error = git_config_get_multivar_foreach(config, "safe.directory", NULL, validate_ownership_cb, &data);
+	return error;
+}
+
+static int validate_ownership(git_repository *repo)
+{
+	const char *validation_paths[3] = { NULL }, *path;
+	size_t validation_len = 0, i;
+	bool is_safe = false;
+	int error = 0;
+
+	/*
+	 * If there's a worktree, validate the permissions to it *and*
+	 * the git directory, and use the worktree as the configuration
+	 * key for allowlisting the directory. In a bare setup, only
+	 * look at the gitdir and use that as the allowlist. So we
+	 * examine all `validation_paths` but use only the first as
+	 * the configuration lookup.
+	 */
+
+	if (repo->workdir)
+		validation_paths[validation_len++] = repo->workdir;
 
-		if (!error && data.is_safe)
+	if (repo->gitlink)
+		validation_paths[validation_len++] = repo->gitlink;
+
+	validation_paths[validation_len++] = repo->gitdir;
+
+	for (i = 0; i < validation_len; i++) {
+		path = validation_paths[i];
+
+		if ((error = validate_ownership_path(&is_safe, path)) < 0)
 			goto done;
+
+		if (!is_safe)
+			break;
 	}
 
-	git_error_set(GIT_ERROR_CONFIG,
-		"repository path '%s' is not owned by current user",
-		repo_path);
-	error = GIT_EOWNER;
+	if (is_safe ||
+	    (error = validate_ownership_config(&is_safe, validation_paths[0])) < 0)
+		goto done;
+
+	if (!is_safe) {
+		git_error_set(GIT_ERROR_CONFIG,
+			"repository path '%s' is not owned by current user",
+			path);
+		error = GIT_EOWNER;
+	}
 
 done:
-	git_config_free(config);
-	git_buf_dispose(&data.tmp);
 	return error;
 }
 
@@ -917,7 +966,6 @@ int git_repository_open_ext(
 		gitlink = GIT_BUF_INIT, commondir = GIT_BUF_INIT;
  	git_repository *repo = NULL;
 	git_config *config = NULL;
-	const char *validation_path;
 
 	if (flags & GIT_REPOSITORY_OPEN_FROM_ENV)
 		return _git_repository_open_ext_from_env(repo_ptr, start_path);
# 	int version = 0;
# 
# 	if (flags & GIT_REPOSITORY_OPEN_FROM_ENV)
@@ -976,12 +1024,11 @@ int git_repository_open_ext(
 	}
 
 	/*
-	 * Ensure that the git directory is owned by the current user.
+	 * Ensure that the git directory and worktree are
+	 * owned by the current user.
 	 */
-	validation_path = repo->is_bare ? repo->gitdir : repo->workdir;
-
 	if (git_repository__validate_ownership &&
-	    (error = validate_ownership(validation_path)) < 0)
+	    (error = validate_ownership(repo)) < 0)
 		goto cleanup;
 
 cleanup:
-- 
2.37.1

