@matheusbsbs / matheus-bagliano / commits / 3b0bf27049

setup: tighten ownership checks post CVE-2022-24765

8959555cee7 (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02), adds a function to check for ownership of repositories using a directory that is representative of it, and ways to add exempt a specific repository from said check if needed, but that check didn't account for owership of the gitdir, or (when used) the gitfile that points to that gitdir. An attacker could create a git repository in a directory that they can write into but that is owned by the victim to work around the fix that was introduced with CVE-2022-24765 to potentially run code as the victim. An example that could result in privilege escalation to root in *NIX would be to set a repository in a shared tmp directory by doing (for example): $ git -C /tmp init To avoid that, extend the ensure_valid_ownership function to be able to check for all three paths. This will have the side effect of tripling the number of stat() calls when a repository is detected, but the effect is expected to be likely minimal, as it is done only once during the directory walk in which Git looks for a repository. Additionally make sure to resolve the gitfile (if one was used) to find the relevant gitdir for checking. While at it change the message printed on failure so it is clear we are referring to the repository by its worktree (or gitdir if it is bare) and not to a specific directory. Helped-by: Junio C Hamano <junio@pobox.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

Carlo Marcelo Arenas Belón committed May 10, 2022 at 12:35 UTC 3b0bf2704980b1ed6018622bdf5377ec22289688

1 file changed +60 -11

setup.c

+60 -11

@@ -1054,14 +1054,32 @@ static int safe_directory_cb(const char *key, const char *value, void *d)

1054 return 0;

1055 }

1056

1057 -static int ensure_valid_ownership(const char *path)

1057 +/*

1058 + * Check if a repository is safe, by verifying the ownership of the

1059 + * worktree (if any), the git directory, and the gitfile (if any).

1060 + *

1061 + * Exemptions for known-safe repositories can be added via `safe.directory`

1062 + * config settings; for non-bare repositories, their worktree needs to be

1063 + * added, for bare ones their git directory.

1064 + */

1065 +static int ensure_valid_ownership(const char *gitfile,

1066 + const char *worktree, const char *gitdir)

1067 {

1059 - struct safe_directory_data data = { .path = path };

1068 + struct safe_directory_data data = {

1069 + .path = worktree ? worktree : gitdir

1070 + };

1071

1072 if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&

1062 - is_path_owned_by_current_user(path))

1073 + (!gitfile || is_path_owned_by_current_user(gitfile)) &&

1074 + (!worktree || is_path_owned_by_current_user(worktree)) &&

1075 + (!gitdir || is_path_owned_by_current_user(gitdir)))

1076 return 1;

1077

1078 + /*

1079 + * data.path is the "path" that identifies the repository and it is

1080 + * constant regardless of what failed above. data.is_safe should be

1081 + * initialized to false, and might be changed by the callback.

1082 + */

1083 read_very_early_config(safe_directory_cb, &data);

1084

1085 return data.is_safe;

@@ -1149,6 +1167,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,

1167 current_device = get_device_or_die(dir->buf, NULL, 0);

1168 for (;;) {

1169 int offset = dir->len, error_code = 0;

1170 + char *gitdir_path = NULL;

1171 + char *gitfile = NULL;

1172

1173 if (offset > min_offset)

1174 strbuf_addch(dir, '/');

@@ -1159,21 +1179,50 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,

1179 if (die_on_error ||

1180 error_code == READ_GITFILE_ERR_NOT_A_FILE) {

1181 /* NEEDSWORK: fail if .git is not file nor dir */

1162 - if (is_git_directory(dir->buf))

1182 + if (is_git_directory(dir->buf)) {

1183 gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;

1184 + gitdir_path = xstrdup(dir->buf);

1185 + }

1186 } else if (error_code != READ_GITFILE_ERR_STAT_FAILED)

1187 return GIT_DIR_INVALID_GITFILE;

1166 - }

1188 + } else

1189 + gitfile = xstrdup(dir->buf);

1190 + /*

1191 + * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT

1192 + * to check that directory for a repository.

1193 + * Now trim that tentative addition away, because we want to

1194 + * focus on the real directory we are in.

1195 + */

1196 strbuf_setlen(dir, offset);

1197 if (gitdirenv) {

1169 - if (!ensure_valid_ownership(dir->buf))

1170 - return GIT_DIR_INVALID_OWNERSHIP;

1171 - strbuf_addstr(gitdir, gitdirenv);

1172 - return GIT_DIR_DISCOVERED;

1198 + enum discovery_result ret;

1199 +

1200 + if (ensure_valid_ownership(gitfile,

1201 + dir->buf,

1202 + (gitdir_path ? gitdir_path : gitdirenv))) {

1203 + strbuf_addstr(gitdir, gitdirenv);

1204 + ret = GIT_DIR_DISCOVERED;

1205 + } else

1206 + ret = GIT_DIR_INVALID_OWNERSHIP;

1207 +

1208 + /*

1209 + * Earlier, during discovery, we might have allocated

1210 + * string copies for gitdir_path or gitfile so make

1211 + * sure we don't leak by freeing them now, before

1212 + * leaving the loop and function.

1213 + *

1214 + * Note: gitdirenv will be non-NULL whenever these are

1215 + * allocated, therefore we need not take care of releasing

1216 + * them outside of this conditional block.

1217 + */

1218 + free(gitdir_path);

1219 + free(gitfile);

1220 +

1221 + return ret;

1222 }

1223

1224 if (is_git_directory(dir->buf)) {

1176 - if (!ensure_valid_ownership(dir->buf))

1225 + if (!ensure_valid_ownership(NULL, NULL, dir->buf))

1226 return GIT_DIR_INVALID_OWNERSHIP;

1227 strbuf_addstr(gitdir, ".");

1228 return GIT_DIR_BARE;

@@ -1306,7 +1355,7 @@ const char *setup_git_directory_gently(int *nongit_ok)

1355 struct strbuf quoted = STRBUF_INIT;

1356

1357 sq_quote_buf_pretty(&quoted, dir.buf);

1309 - die(_("unsafe repository ('%s' is owned by someone else)\n"

1358 + die(_("detected dubious ownership in repository at '%s'\n"

1359 "To add an exception for this directory, call:\n"

1360 "\n"

1361 "\tgit config --global --add safe.directory %s"),