Commit af9e00321f06383ccf61f4ca9616716bec0eb2da

Edward Thomson 2022-07-02T10:19:33

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.

diff --git a/src/libgit2/repository.c b/src/libgit2/repository.c
index 09e81cd..17aec00 100644
--- a/src/libgit2/repository.c
+++ b/src/libgit2/repository.c
@@ -488,7 +488,7 @@ static int read_gitfile(git_str *path_out, const char *file_path)
 typedef struct {
 	const char *repo_path;
 	git_str tmp;
-	bool is_safe;
+	bool *is_safe;
 } validate_ownership_data;
 
 static int validate_ownership_cb(const git_config_entry *entry, void *payload)
@@ -496,52 +496,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_fs_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_STR_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_str_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_STR_INIT, false };
 	git_fs_path_owner_t owner_level =
 		GIT_FS_PATH_OWNER_CURRENT_USER |
 		GIT_FS_PATH_USER_IS_ADMINISTRATOR;
-	bool is_safe;
-	int error;
-
-	if ((error = git_fs_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_fs_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_str_dispose(&data.tmp);
 	return error;
 }
 
@@ -918,7 +967,6 @@ int git_repository_open_ext(
 		gitlink = GIT_STR_INIT, commondir = GIT_STR_INIT;
 	git_repository *repo = NULL;
 	git_config *config = NULL;
-	const char *validation_path;
 	int version = 0;
 
 	if (flags & GIT_REPOSITORY_OPEN_FROM_ENV)
@@ -977,12 +1025,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: