Skip to content

Commit

Permalink
Sync with 2.33.4
Browse files Browse the repository at this point in the history
* maint-2.33:
  Git 2.33.4
  Git 2.32.3
  Git 2.31.4
  Git 2.30.5
  setup: tighten ownership checks post CVE-2022-24765
  git-compat-util: allow root to access both SUDO_UID and root owned
  t0034: add negative tests and allow git init to mostly work under sudo
  git-compat-util: avoid failing dir ownership checks if running privileged
  t: regression git needs safe.directory when using sudo
  • Loading branch information
dscho committed Jun 23, 2022
2 parents 2f0dde7 + 80c525c commit 378eade
Show file tree
Hide file tree
Showing 9 changed files with 269 additions and 12 deletions.
12 changes: 12 additions & 0 deletions Documentation/RelNotes/2.30.5.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Git v2.30.5 Release Notes
=========================

This release contains minor fix-ups for the changes that went into
Git 2.30.3 and 2.30.4, addressing CVE-2022-29187.

* The safety check that verifies a safe ownership of the Git
worktree is now extended to also cover the ownership of the Git
directory (and the `.git` file, if there is any).

Carlo Marcelo Arenas Belón (1):
setup: tighten ownership checks post CVE-2022-24765
6 changes: 6 additions & 0 deletions Documentation/RelNotes/2.31.4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Git v2.31.4 Release Notes
=========================

This release merges up the fixes that appear in v2.30.5 to address
the security issue CVE-2022-29187; see the release notes for that
version for details.
6 changes: 6 additions & 0 deletions Documentation/RelNotes/2.32.3.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Git v2.32.3 Release Notes
=========================

This release merges up the fixes that appear in v2.30.5 and
v2.31.4 to address the security issue CVE-2022-29187; see the
release notes for these versions for details.
6 changes: 6 additions & 0 deletions Documentation/RelNotes/2.33.4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Git v2.33.4 Release Notes
=========================

This release merges up the fixes that appear in v2.30.5, v2.31.4
and v2.32.3 to address the security issue CVE-2022-29187; see
the release notes for these versions for details.
14 changes: 14 additions & 0 deletions Documentation/config/safe.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,17 @@ directory was listed in the `safe.directory` list. If `safe.directory=*`
is set in system config and you want to re-enable this protection, then
initialize your list with an empty value before listing the repositories
that you deem safe.
+
As explained, Git only allows you to access repositories owned by
yourself, i.e. the user who is running Git, by default. When Git
is running as 'root' in a non Windows platform that provides sudo,
however, git checks the SUDO_UID environment variable that sudo creates
and will allow access to the uid recorded as its value in addition to
the id from 'root'.
This is to make it easy to perform a common sequence during installation
"make && sudo make install". A git process running under 'sudo' runs as
'root' but the 'sudo' command exports the environment variable to record
which id the original user has.
If that is not what you would prefer and want git to only trust
repositories that are owned by root instead, then you can remove
the `SUDO_UID` variable from root's environment before invoking git.
58 changes: 57 additions & 1 deletion git-compat-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,12 +401,68 @@ static inline int git_offset_1st_component(const char *path)
#endif

#ifndef is_path_owned_by_current_user

#ifdef __TANDEM
#define ROOT_UID 65535
#else
#define ROOT_UID 0
#endif

/*
* Do not use this function when
* (1) geteuid() did not say we are running as 'root', or
* (2) using this function will compromise the system.
*
* PORTABILITY WARNING:
* This code assumes uid_t is unsigned because that is what sudo does.
* If your uid_t type is signed and all your ids are positive then it
* should all work fine.
* If your version of sudo uses negative values for uid_t or it is
* buggy and return an overflowed value in SUDO_UID, then git might
* fail to grant access to your repository properly or even mistakenly
* grant access to someone else.
* In the unlikely scenario this happened to you, and that is how you
* got to this message, we would like to know about it; so sent us an
* email to [email protected] indicating which platform you are
* using and which version of sudo, so we can improve this logic and
* maybe provide you with a patch that would prevent this issue again
* in the future.
*/
static inline void extract_id_from_env(const char *env, uid_t *id)
{
const char *real_uid = getenv(env);

/* discard anything empty to avoid a more complex check below */
if (real_uid && *real_uid) {
char *endptr = NULL;
unsigned long env_id;

errno = 0;
/* silent overflow errors could trigger a bug here */
env_id = strtoul(real_uid, &endptr, 10);
if (!*endptr && !errno)
*id = env_id;
}
}

static inline int is_path_owned_by_current_uid(const char *path)
{
struct stat st;
uid_t euid;

if (lstat(path, &st))
return 0;
return st.st_uid == geteuid();

euid = geteuid();
if (euid == ROOT_UID)
{
if (st.st_uid == ROOT_UID)
return 1;
else
extract_id_from_env("SUDO_UID", &euid);
}

return st.st_uid == euid;
}

#define is_path_owned_by_current_user is_path_owned_by_current_uid
Expand Down
71 changes: 60 additions & 11 deletions setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1055,14 +1055,32 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
return 0;
}

static int ensure_valid_ownership(const char *path)
/*
* Check if a repository is safe, by verifying the ownership of the
* worktree (if any), the git directory, and the gitfile (if any).
*
* Exemptions for known-safe repositories can be added via `safe.directory`
* config settings; for non-bare repositories, their worktree needs to be
* added, for bare ones their git directory.
*/
static int ensure_valid_ownership(const char *gitfile,
const char *worktree, const char *gitdir)
{
struct safe_directory_data data = { .path = path };
struct safe_directory_data data = {
.path = worktree ? worktree : gitdir
};

if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
is_path_owned_by_current_user(path))
(!gitfile || is_path_owned_by_current_user(gitfile)) &&
(!worktree || is_path_owned_by_current_user(worktree)) &&
(!gitdir || is_path_owned_by_current_user(gitdir)))
return 1;

/*
* data.path is the "path" that identifies the repository and it is
* constant regardless of what failed above. data.is_safe should be
* initialized to false, and might be changed by the callback.
*/
read_very_early_config(safe_directory_cb, &data);

return data.is_safe;
Expand Down Expand Up @@ -1150,6 +1168,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
int offset = dir->len, error_code = 0;
char *gitdir_path = NULL;
char *gitfile = NULL;

if (offset > min_offset)
strbuf_addch(dir, '/');
Expand All @@ -1160,21 +1180,50 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (die_on_error ||
error_code == READ_GITFILE_ERR_NOT_A_FILE) {
/* NEEDSWORK: fail if .git is not file nor dir */
if (is_git_directory(dir->buf))
if (is_git_directory(dir->buf)) {
gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
gitdir_path = xstrdup(dir->buf);
}
} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)
return GIT_DIR_INVALID_GITFILE;
}
} else
gitfile = xstrdup(dir->buf);
/*
* Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT
* to check that directory for a repository.
* Now trim that tentative addition away, because we want to
* focus on the real directory we are in.
*/
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;
enum discovery_result ret;

if (ensure_valid_ownership(gitfile,
dir->buf,
(gitdir_path ? gitdir_path : gitdirenv))) {
strbuf_addstr(gitdir, gitdirenv);
ret = GIT_DIR_DISCOVERED;
} else
ret = GIT_DIR_INVALID_OWNERSHIP;

/*
* Earlier, during discovery, we might have allocated
* string copies for gitdir_path or gitfile so make
* sure we don't leak by freeing them now, before
* leaving the loop and function.
*
* Note: gitdirenv will be non-NULL whenever these are
* allocated, therefore we need not take care of releasing
* them outside of this conditional block.
*/
free(gitdir_path);
free(gitfile);

return ret;
}

if (is_git_directory(dir->buf)) {
if (!ensure_valid_ownership(dir->buf))
if (!ensure_valid_ownership(NULL, NULL, dir->buf))
return GIT_DIR_INVALID_OWNERSHIP;
strbuf_addstr(gitdir, ".");
return GIT_DIR_BARE;
Expand Down Expand Up @@ -1312,7 +1361,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
struct strbuf quoted = STRBUF_INIT;

sq_quote_buf_pretty(&quoted, dir.buf);
die(_("unsafe repository ('%s' is owned by someone else)\n"
die(_("detected dubious ownership in repository at '%s'\n"
"To add an exception for this directory, call:\n"
"\n"
"\tgit config --global --add safe.directory %s"),
Expand Down
15 changes: 15 additions & 0 deletions t/lib-sudo.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Helpers for running git commands under sudo.

# Runs a scriplet passed through stdin under sudo.
run_with_sudo () {
local ret
local RUN="$TEST_DIRECTORY/$$.sh"
write_script "$RUN" "$TEST_SHELL_PATH"
# avoid calling "$RUN" directly so sudo doesn't get a chance to
# override the shell, add aditional restrictions or even reject
# running the script because its security policy deem it unsafe
sudo "$TEST_SHELL_PATH" -c "\"$RUN\""
ret=$?
rm -f "$RUN"
return $ret
}
93 changes: 93 additions & 0 deletions t/t0034-root-safe-directory.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#!/bin/sh

test_description='verify safe.directory checks while running as root'

. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-sudo.sh

if [ "$GIT_TEST_ALLOW_SUDO" != "YES" ]
then
skip_all="You must set env var GIT_TEST_ALLOW_SUDO=YES in order to run this test"
test_done
fi

if ! test_have_prereq NOT_ROOT
then
skip_all="These tests do not support running as root"
test_done
fi

test_lazy_prereq SUDO '
sudo -n id -u >u &&
id -u root >r &&
test_cmp u r &&
command -v git >u &&
sudo command -v git >r &&
test_cmp u r
'

if ! test_have_prereq SUDO
then
skip_all="Your sudo/system configuration is either too strict or unsupported"
test_done
fi

test_expect_success SUDO 'setup' '
sudo rm -rf root &&
mkdir -p root/r &&
(
cd root/r &&
git init
)
'

test_expect_success SUDO 'sudo git status as original owner' '
(
cd root/r &&
git status &&
sudo git status
)
'

test_expect_success SUDO 'setup root owned repository' '
sudo mkdir -p root/p &&
sudo git init root/p
'

test_expect_success 'cannot access if owned by root' '
(
cd root/p &&
test_must_fail git status
)
'

test_expect_success 'can access if addressed explicitly' '
(
cd root/p &&
GIT_DIR=.git GIT_WORK_TREE=. git status
)
'

test_expect_success SUDO 'can access with sudo if root' '
(
cd root/p &&
sudo git status
)
'

test_expect_success SUDO 'can access with sudo if root by removing SUDO_UID' '
(
cd root/p &&
run_with_sudo <<-END
unset SUDO_UID &&
git status
END
)
'

# this MUST be always the last test
test_expect_success SUDO 'cleanup' '
sudo rm -rf root
'

test_done

0 comments on commit 378eade

Please sign in to comment.