setup_git_directory(): add an owner check for the top-level directory
It poses a security risk to search for a git directory outside of the directories owned by the current user. For example, it is common e.g. in computer pools of educational institutes to have a "scratch" space: a mounted disk with plenty of space that is regularly swiped where any authenticated user can create a directory to do their work. Merely navigating to such a space with a Git-enabled `PS1` when there is a maliciously-crafted `/scratch/.git/` can lead to a compromised account. The same holds true in multi-user setups running Windows, as `C:\` is writable to every authenticated user by default. To plug this vulnerability, we stop Git from accepting top-level directories owned by someone other than the current user. We avoid looking at the ownership of each and every directories between the current and the top-level one (if there are any between) to avoid introducing a performance bottleneck. This new default behavior is obviously incompatible with the concept of shared repositories, where we expect the top-level directory to be owned by only one of its legitimate users. To re-enable that use case, we add support for adding exceptions from the new default behavior via the config setting `safe.directory`. The `safe.directory` config setting is only respected in the system and global configs, not from repository configs or via the command-line, and can have multiple values to allow for multiple shared repositories. We are particularly careful to provide a helpful message to any user trying to use a shared repository. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>maint
							parent
							
								
									bdc77d1d68
								
							
						
					
					
						commit
						8959555cee
					
				|  | @ -438,6 +438,8 @@ include::config/rerere.txt[] | ||||||
|  |  | ||||||
| include::config/reset.txt[] | include::config/reset.txt[] | ||||||
|  |  | ||||||
|  | include::config/safe.txt[] | ||||||
|  |  | ||||||
| include::config/sendemail.txt[] | include::config/sendemail.txt[] | ||||||
|  |  | ||||||
| include::config/sequencer.txt[] | include::config/sequencer.txt[] | ||||||
|  |  | ||||||
|  | @ -0,0 +1,21 @@ | ||||||
|  | safe.directory:: | ||||||
|  | 	These config entries specify Git-tracked directories that are | ||||||
|  | 	considered safe even if they are owned by someone other than the | ||||||
|  | 	current user. By default, Git will refuse to even parse a Git | ||||||
|  | 	config of a repository owned by someone else, let alone run its | ||||||
|  | 	hooks, and this config setting allows users to specify exceptions, | ||||||
|  | 	e.g. for intentionally shared repositories (see the `--shared` | ||||||
|  | 	option in linkgit:git-init[1]). | ||||||
|  | + | ||||||
|  | This is a multi-valued setting, i.e. you can add more than one directory | ||||||
|  | via `git config --add`. To reset the list of safe directories (e.g. to | ||||||
|  | override any such directories specified in the system config), add a | ||||||
|  | `safe.directory` entry with an empty value. | ||||||
|  | + | ||||||
|  | This config setting is only respected when specified in a system or global | ||||||
|  | config, not when it is specified in a repository config or via the command | ||||||
|  | line option `-c safe.directory=<path>`. | ||||||
|  | + | ||||||
|  | The value of this setting is interpolated, i.e. `~/<path>` expands to a | ||||||
|  | path relative to the home directory and `%(prefix)/<path>` expands to a | ||||||
|  | path relative to Git's (runtime) prefix. | ||||||
							
								
								
									
										57
									
								
								setup.c
								
								
								
								
							
							
						
						
									
										57
									
								
								setup.c
								
								
								
								
							|  | @ -5,6 +5,7 @@ | ||||||
| #include "string-list.h" | #include "string-list.h" | ||||||
| #include "chdir-notify.h" | #include "chdir-notify.h" | ||||||
| #include "promisor-remote.h" | #include "promisor-remote.h" | ||||||
|  | #include "quote.h" | ||||||
|  |  | ||||||
| static int inside_git_dir = -1; | static int inside_git_dir = -1; | ||||||
| static int inside_work_tree = -1; | static int inside_work_tree = -1; | ||||||
|  | @ -1024,6 +1025,42 @@ static int canonicalize_ceiling_entry(struct string_list_item *item, | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
|  | struct safe_directory_data { | ||||||
|  | 	const char *path; | ||||||
|  | 	int is_safe; | ||||||
|  | }; | ||||||
|  |  | ||||||
|  | static int safe_directory_cb(const char *key, const char *value, void *d) | ||||||
|  | { | ||||||
|  | 	struct safe_directory_data *data = d; | ||||||
|  |  | ||||||
|  | 	if (!value || !*value) | ||||||
|  | 		data->is_safe = 0; | ||||||
|  | 	else { | ||||||
|  | 		const char *interpolated = NULL; | ||||||
|  |  | ||||||
|  | 		if (!git_config_pathname(&interpolated, key, value) && | ||||||
|  | 		    !fspathcmp(data->path, interpolated ? interpolated : value)) | ||||||
|  | 			data->is_safe = 1; | ||||||
|  |  | ||||||
|  | 		free((char *)interpolated); | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	return 0; | ||||||
|  | } | ||||||
|  |  | ||||||
|  | static int ensure_valid_ownership(const char *path) | ||||||
|  | { | ||||||
|  | 	struct safe_directory_data data = { .path = path }; | ||||||
|  |  | ||||||
|  | 	if (is_path_owned_by_current_user(path)) | ||||||
|  | 		return 1; | ||||||
|  |  | ||||||
|  | 	read_very_early_config(safe_directory_cb, &data); | ||||||
|  |  | ||||||
|  | 	return data.is_safe; | ||||||
|  | } | ||||||
|  |  | ||||||
| enum discovery_result { | enum discovery_result { | ||||||
| 	GIT_DIR_NONE = 0, | 	GIT_DIR_NONE = 0, | ||||||
| 	GIT_DIR_EXPLICIT, | 	GIT_DIR_EXPLICIT, | ||||||
|  | @ -1032,7 +1069,8 @@ enum discovery_result { | ||||||
| 	/* these are errors */ | 	/* these are errors */ | ||||||
| 	GIT_DIR_HIT_CEILING = -1, | 	GIT_DIR_HIT_CEILING = -1, | ||||||
| 	GIT_DIR_HIT_MOUNT_POINT = -2, | 	GIT_DIR_HIT_MOUNT_POINT = -2, | ||||||
| 	GIT_DIR_INVALID_GITFILE = -3 | 	GIT_DIR_INVALID_GITFILE = -3, | ||||||
|  | 	GIT_DIR_INVALID_OWNERSHIP = -4 | ||||||
| }; | }; | ||||||
|  |  | ||||||
| /* | /* | ||||||
|  | @ -1122,11 +1160,15 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, | ||||||
| 		} | 		} | ||||||
| 		strbuf_setlen(dir, offset); | 		strbuf_setlen(dir, offset); | ||||||
| 		if (gitdirenv) { | 		if (gitdirenv) { | ||||||
|  | 			if (!ensure_valid_ownership(dir->buf)) | ||||||
|  | 				return GIT_DIR_INVALID_OWNERSHIP; | ||||||
| 			strbuf_addstr(gitdir, gitdirenv); | 			strbuf_addstr(gitdir, gitdirenv); | ||||||
| 			return GIT_DIR_DISCOVERED; | 			return GIT_DIR_DISCOVERED; | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
| 		if (is_git_directory(dir->buf)) { | 		if (is_git_directory(dir->buf)) { | ||||||
|  | 			if (!ensure_valid_ownership(dir->buf)) | ||||||
|  | 				return GIT_DIR_INVALID_OWNERSHIP; | ||||||
| 			strbuf_addstr(gitdir, "."); | 			strbuf_addstr(gitdir, "."); | ||||||
| 			return GIT_DIR_BARE; | 			return GIT_DIR_BARE; | ||||||
| 		} | 		} | ||||||
|  | @ -1253,6 +1295,19 @@ const char *setup_git_directory_gently(int *nongit_ok) | ||||||
| 			    dir.buf); | 			    dir.buf); | ||||||
| 		*nongit_ok = 1; | 		*nongit_ok = 1; | ||||||
| 		break; | 		break; | ||||||
|  | 	case GIT_DIR_INVALID_OWNERSHIP: | ||||||
|  | 		if (!nongit_ok) { | ||||||
|  | 			struct strbuf quoted = STRBUF_INIT; | ||||||
|  |  | ||||||
|  | 			sq_quote_buf_pretty("ed, dir.buf); | ||||||
|  | 			die(_("unsafe repository ('%s' is owned by someone else)\n" | ||||||
|  | 			      "To add an exception for this directory, call:\n" | ||||||
|  | 			      "\n" | ||||||
|  | 			      "\tgit config --global --add safe.directory %s"), | ||||||
|  | 			    dir.buf, quoted.buf); | ||||||
|  | 		} | ||||||
|  | 		*nongit_ok = 1; | ||||||
|  | 		break; | ||||||
| 	case GIT_DIR_NONE: | 	case GIT_DIR_NONE: | ||||||
| 		/* | 		/* | ||||||
| 		 * As a safeguard against setup_git_directory_gently_1 returning | 		 * As a safeguard against setup_git_directory_gently_1 returning | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Johannes Schindelin
						Johannes Schindelin