From 8959555cee7ec045958f9b6dd62e541affb7e7d9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 2 Mar 2022 12:23:04 +0100 Subject: [PATCH] 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 --- Documentation/config.txt | 2 ++ Documentation/config/safe.txt | 21 ++++++++++++++++ setup.c | 57 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 Documentation/config/safe.txt diff --git a/Documentation/config.txt b/Documentation/config.txt index 6ba50b1104..34e6d477d6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -438,6 +438,8 @@ include::config/rerere.txt[] include::config/reset.txt[] +include::config/safe.txt[] + include::config/sendemail.txt[] include::config/sequencer.txt[] diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt new file mode 100644 index 0000000000..63597b2df8 --- /dev/null +++ b/Documentation/config/safe.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=`. ++ +The value of this setting is interpolated, i.e. `~/` expands to a +path relative to the home directory and `%(prefix)/` expands to a +path relative to Git's (runtime) prefix. diff --git a/setup.c b/setup.c index c04cd25a30..95d5b00940 100644 --- a/setup.c +++ b/setup.c @@ -5,6 +5,7 @@ #include "string-list.h" #include "chdir-notify.h" #include "promisor-remote.h" +#include "quote.h" static int inside_git_dir = -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 { GIT_DIR_NONE = 0, GIT_DIR_EXPLICIT, @@ -1032,7 +1069,8 @@ enum discovery_result { /* these are errors */ GIT_DIR_HIT_CEILING = -1, 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); if (gitdirenv) { + if (!ensure_valid_ownership(dir->buf)) + return GIT_DIR_INVALID_OWNERSHIP; strbuf_addstr(gitdir, gitdirenv); return GIT_DIR_DISCOVERED; } if (is_git_directory(dir->buf)) { + if (!ensure_valid_ownership(dir->buf)) + return GIT_DIR_INVALID_OWNERSHIP; strbuf_addstr(gitdir, "."); return GIT_DIR_BARE; } @@ -1253,6 +1295,19 @@ const char *setup_git_directory_gently(int *nongit_ok) dir.buf); *nongit_ok = 1; 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: /* * As a safeguard against setup_git_directory_gently_1 returning -- 2.11.4.GIT