Skip to content

Commit 8959555

Browse files
committed
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 <[email protected]>
1 parent bdc77d1 commit 8959555

File tree

3 files changed

+79
-1
lines changed

3 files changed

+79
-1
lines changed

Documentation/config.txt

+2
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,8 @@ include::config/rerere.txt[]
438438

439439
include::config/reset.txt[]
440440

441+
include::config/safe.txt[]
442+
441443
include::config/sendemail.txt[]
442444

443445
include::config/sequencer.txt[]

Documentation/config/safe.txt

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
safe.directory::
2+
These config entries specify Git-tracked directories that are
3+
considered safe even if they are owned by someone other than the
4+
current user. By default, Git will refuse to even parse a Git
5+
config of a repository owned by someone else, let alone run its
6+
hooks, and this config setting allows users to specify exceptions,
7+
e.g. for intentionally shared repositories (see the `--shared`
8+
option in linkgit:git-init[1]).
9+
+
10+
This is a multi-valued setting, i.e. you can add more than one directory
11+
via `git config --add`. To reset the list of safe directories (e.g. to
12+
override any such directories specified in the system config), add a
13+
`safe.directory` entry with an empty value.
14+
+
15+
This config setting is only respected when specified in a system or global
16+
config, not when it is specified in a repository config or via the command
17+
line option `-c safe.directory=<path>`.
18+
+
19+
The value of this setting is interpolated, i.e. `~/<path>` expands to a
20+
path relative to the home directory and `%(prefix)/<path>` expands to a
21+
path relative to Git's (runtime) prefix.

setup.c

+56-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "string-list.h"
66
#include "chdir-notify.h"
77
#include "promisor-remote.h"
8+
#include "quote.h"
89

910
static int inside_git_dir = -1;
1011
static int inside_work_tree = -1;
@@ -1024,6 +1025,42 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
10241025
}
10251026
}
10261027

1028+
struct safe_directory_data {
1029+
const char *path;
1030+
int is_safe;
1031+
};
1032+
1033+
static int safe_directory_cb(const char *key, const char *value, void *d)
1034+
{
1035+
struct safe_directory_data *data = d;
1036+
1037+
if (!value || !*value)
1038+
data->is_safe = 0;
1039+
else {
1040+
const char *interpolated = NULL;
1041+
1042+
if (!git_config_pathname(&interpolated, key, value) &&
1043+
!fspathcmp(data->path, interpolated ? interpolated : value))
1044+
data->is_safe = 1;
1045+
1046+
free((char *)interpolated);
1047+
}
1048+
1049+
return 0;
1050+
}
1051+
1052+
static int ensure_valid_ownership(const char *path)
1053+
{
1054+
struct safe_directory_data data = { .path = path };
1055+
1056+
if (is_path_owned_by_current_user(path))
1057+
return 1;
1058+
1059+
read_very_early_config(safe_directory_cb, &data);
1060+
1061+
return data.is_safe;
1062+
}
1063+
10271064
enum discovery_result {
10281065
GIT_DIR_NONE = 0,
10291066
GIT_DIR_EXPLICIT,
@@ -1032,7 +1069,8 @@ enum discovery_result {
10321069
/* these are errors */
10331070
GIT_DIR_HIT_CEILING = -1,
10341071
GIT_DIR_HIT_MOUNT_POINT = -2,
1035-
GIT_DIR_INVALID_GITFILE = -3
1072+
GIT_DIR_INVALID_GITFILE = -3,
1073+
GIT_DIR_INVALID_OWNERSHIP = -4
10361074
};
10371075

10381076
/*
@@ -1122,11 +1160,15 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
11221160
}
11231161
strbuf_setlen(dir, offset);
11241162
if (gitdirenv) {
1163+
if (!ensure_valid_ownership(dir->buf))
1164+
return GIT_DIR_INVALID_OWNERSHIP;
11251165
strbuf_addstr(gitdir, gitdirenv);
11261166
return GIT_DIR_DISCOVERED;
11271167
}
11281168

11291169
if (is_git_directory(dir->buf)) {
1170+
if (!ensure_valid_ownership(dir->buf))
1171+
return GIT_DIR_INVALID_OWNERSHIP;
11301172
strbuf_addstr(gitdir, ".");
11311173
return GIT_DIR_BARE;
11321174
}
@@ -1253,6 +1295,19 @@ const char *setup_git_directory_gently(int *nongit_ok)
12531295
dir.buf);
12541296
*nongit_ok = 1;
12551297
break;
1298+
case GIT_DIR_INVALID_OWNERSHIP:
1299+
if (!nongit_ok) {
1300+
struct strbuf quoted = STRBUF_INIT;
1301+
1302+
sq_quote_buf_pretty(&quoted, dir.buf);
1303+
die(_("unsafe repository ('%s' is owned by someone else)\n"
1304+
"To add an exception for this directory, call:\n"
1305+
"\n"
1306+
"\tgit config --global --add safe.directory %s"),
1307+
dir.buf, quoted.buf);
1308+
}
1309+
*nongit_ok = 1;
1310+
break;
12561311
case GIT_DIR_NONE:
12571312
/*
12581313
* As a safeguard against setup_git_directory_gently_1 returning

0 commit comments

Comments
 (0)