Re: [PATCH] [RFC] setup.c: make bare repo discovery optional
From: Junio C Hamano <hidden>
Date: 2022-05-06 20:33:38
"Glen Choo via GitGitGadget" [off-list ref] writes:
From: Glen Choo <redacted> Add a config variable, `safe.barerepository`, that tells Git whether or not to recognize bare repositories when it is trying to discover the repository. This only affects repository discovery, thus it has no effect if discovery was not done (e.g. `--git-dir` was passed).
+safe.barerepository:: + This config entry specifies directories that Git can recognize as + a bare repository when looking for the repository (aka repository + discovery). This has no effect if repository discovery is not + performed e.g. the path to the repository is set via `--git-dir` + (see linkgit:git[1]). ++ +It is recommended that you set this value so that Git will only use the bare +repositories you intend it to. This prevents certain types of security and +non-security problems, such as: + +* `git clone`-ing a repository containing a maliciously bare repository + inside it.
"maliciously bare"? "malicious bare" probably.
+* Git recognizing a directory that isn't mean to be a bare repository,
"mean to be" -> "meant to be".
+ but happens to look like one.
quoted hunk
diff --git a/setup.c b/setup.c index a7b36f3ffbf..9b5dd877273 100644 --- a/setup.c +++ b/setup.c@@ -1133,6 +1133,40 @@ static int ensure_valid_ownership(const char *path) return data.is_safe; } +/* + * This is similar to safe_directory_data, but only supports true/false. + */ +struct safe_bare_repository_data { + int is_safe; +}; + +static int safe_bare_repository_cb(const char *key, const char *value, void *d) +{ + struct safe_bare_repository_data *data = d; + + if (strcmp(key, "safe.barerepository")) + return 0; + + if (!value || !strcmp(value, "*")) { + data->is_safe = 1; + return 0; + } + if (!*value) { + data->is_safe = 0; + return 0; + } + return -1; +} + +static int should_detect_bare(void) +{ + struct safe_bare_repository_data data; + + read_very_early_config(safe_bare_repository_cb, &data); + + return data.is_safe; +} + enum discovery_result { GIT_DIR_NONE = 0, GIT_DIR_EXPLICIT,@@ -1238,7 +1272,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, return GIT_DIR_DISCOVERED; } - if (is_git_directory(dir->buf)) { + if (should_detect_bare() && is_git_directory(dir->buf)) { if (!ensure_valid_ownership(dir->buf)) return GIT_DIR_INVALID_OWNERSHIP; strbuf_addstr(gitdir, ".");
This is in a loop, which will go up and try the parent directory if
the body of this block is not entered, so it is calling the new
should_detect_bare() helper over and over if it returns false.
Not a very good idea.
Perhaps this would help? I dunno.
static int should_detect_bare(void)
{
static int should = -1; /* unknown yet */
if (should < 0) {
struct safe_bare_repository_data data = { 0 };
read_very_early_config(safe_bare_repository_cb, &data);
should = data.is_safe;
}
return should;
}
In any case, I very much appreciate the fact that this touches the
setup_git_directory_gently_1() codepath only minimally, as we have
other plans to update the code further soonish.
Thanks.