Skip to content

Commit af9e003

Browse files
committed
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.
1 parent d1001fd commit af9e003

File tree

1 file changed

+77
-30
lines changed

1 file changed

+77
-30
lines changed

src/libgit2/repository.c

Lines changed: 77 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -488,60 +488,109 @@ static int read_gitfile(git_str *path_out, const char *file_path)
488488
typedef struct {
489489
const char *repo_path;
490490
git_str tmp;
491-
bool is_safe;
491+
bool *is_safe;
492492
} validate_ownership_data;
493493

494494
static int validate_ownership_cb(const git_config_entry *entry, void *payload)
495495
{
496496
validate_ownership_data *data = payload;
497497

498498
if (strcmp(entry->value, "") == 0)
499-
data->is_safe = false;
499+
*data->is_safe = false;
500500

501501
if (git_fs_path_prettify_dir(&data->tmp, entry->value, NULL) == 0 &&
502502
strcmp(data->tmp.ptr, data->repo_path) == 0)
503-
data->is_safe = true;
503+
*data->is_safe = true;
504504

505505
return 0;
506506
}
507507

508-
static int validate_ownership(const char *repo_path)
508+
static int validate_ownership_config(bool *is_safe, const char *path)
509+
{
510+
validate_ownership_data ownership_data = {
511+
path, GIT_STR_INIT, is_safe
512+
};
513+
git_config *config;
514+
int error;
515+
516+
if (load_global_config(&config) != 0)
517+
return 0;
518+
519+
error = git_config_get_multivar_foreach(config,
520+
"safe.directory", NULL,
521+
validate_ownership_cb,
522+
&ownership_data);
523+
524+
git_config_free(config);
525+
git_str_dispose(&ownership_data.tmp);
526+
527+
return error;
528+
}
529+
530+
static int validate_ownership_path(bool *is_safe, const char *path)
509531
{
510-
git_config *config = NULL;
511-
validate_ownership_data data = { repo_path, GIT_STR_INIT, false };
512532
git_fs_path_owner_t owner_level =
513533
GIT_FS_PATH_OWNER_CURRENT_USER |
514534
GIT_FS_PATH_USER_IS_ADMINISTRATOR;
515-
bool is_safe;
516-
int error;
517-
518-
if ((error = git_fs_path_owner_is(&is_safe, repo_path, owner_level)) < 0) {
519-
if (error == GIT_ENOTFOUND)
520-
error = 0;
535+
int error = 0;
521536

522-
goto done;
523-
}
537+
if (path)
538+
error = git_fs_path_owner_is(is_safe, path, owner_level);
524539

525-
if (is_safe) {
540+
if (error == GIT_ENOTFOUND) {
541+
*is_safe = true;
526542
error = 0;
527-
goto done;
528543
}
529544

530-
if (load_global_config(&config) == 0) {
531-
error = git_config_get_multivar_foreach(config, "safe.directory", NULL, validate_ownership_cb, &data);
545+
return error;
546+
}
547+
548+
static int validate_ownership(git_repository *repo)
549+
{
550+
const char *validation_paths[3] = { NULL }, *path;
551+
size_t validation_len = 0, i;
552+
bool is_safe = false;
553+
int error = 0;
554+
555+
/*
556+
* If there's a worktree, validate the permissions to it *and*
557+
* the git directory, and use the worktree as the configuration
558+
* key for allowlisting the directory. In a bare setup, only
559+
* look at the gitdir and use that as the allowlist. So we
560+
* examine all `validation_paths` but use only the first as
561+
* the configuration lookup.
562+
*/
563+
564+
if (repo->workdir)
565+
validation_paths[validation_len++] = repo->workdir;
532566

533-
if (!error && data.is_safe)
567+
if (repo->gitlink)
568+
validation_paths[validation_len++] = repo->gitlink;
569+
570+
validation_paths[validation_len++] = repo->gitdir;
571+
572+
for (i = 0; i < validation_len; i++) {
573+
path = validation_paths[i];
574+
575+
if ((error = validate_ownership_path(&is_safe, path)) < 0)
534576
goto done;
577+
578+
if (!is_safe)
579+
break;
535580
}
536581

537-
git_error_set(GIT_ERROR_CONFIG,
538-
"repository path '%s' is not owned by current user",
539-
repo_path);
540-
error = GIT_EOWNER;
582+
if (is_safe ||
583+
(error = validate_ownership_config(&is_safe, validation_paths[0])) < 0)
584+
goto done;
585+
586+
if (!is_safe) {
587+
git_error_set(GIT_ERROR_CONFIG,
588+
"repository path '%s' is not owned by current user",
589+
path);
590+
error = GIT_EOWNER;
591+
}
541592

542593
done:
543-
git_config_free(config);
544-
git_str_dispose(&data.tmp);
545594
return error;
546595
}
547596

@@ -918,7 +967,6 @@ int git_repository_open_ext(
918967
gitlink = GIT_STR_INIT, commondir = GIT_STR_INIT;
919968
git_repository *repo = NULL;
920969
git_config *config = NULL;
921-
const char *validation_path;
922970
int version = 0;
923971

924972
if (flags & GIT_REPOSITORY_OPEN_FROM_ENV)
@@ -977,12 +1025,11 @@ int git_repository_open_ext(
9771025
}
9781026

9791027
/*
980-
* Ensure that the git directory is owned by the current user.
1028+
* Ensure that the git directory and worktree are
1029+
* owned by the current user.
9811030
*/
982-
validation_path = repo->is_bare ? repo->gitdir : repo->workdir;
983-
9841031
if (git_repository__validate_ownership &&
985-
(error = validate_ownership(validation_path)) < 0)
1032+
(error = validate_ownership(repo)) < 0)
9861033
goto cleanup;
9871034

9881035
cleanup:

0 commit comments

Comments
 (0)