File fix-CVE-2022-24765.patch of Package git

Overview Repositories Revisions Requests Users Attributes Meta

File fix-CVE-2022-24765.patch of Package git (Revision 23c92343b85ac3ec9777b356c4c043db)

Currently displaying revision 23c92343b85ac3ec9777b356c4c043db , Show latest

From bdc77d1d685be9c10b88abb281a42bc620548595 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 2 Mar 2022 11:06:24 +0100
Subject: [PATCH 1/2] Add a function to determine whether a path is owned by
 the current user

This function will be used in the next commit to prevent
`setup_git_directory()` from discovering a repository in a directory
that is owned by someone other than the current user.

Note: We cannot simply use `st.st_uid` on Windows just like we do on
Linux and other Unix-like platforms: according to
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions
this field is always zero on Windows (because Windows' idea of a user ID
does not fit into a single numerical value). Therefore, we have to do
something a little involved to replicate the same functionality there.

Also note: On Windows, a user's home directory is not actually owned by
said user, but by the administrator. For all practical purposes, it is
under the user's control, though, therefore we pretend that it is owned
by the user.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c    | 87 +++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h    |  7 ++++
 git-compat-util.h | 12 +++++++
 3 files changed, 106 insertions(+)

diff --git a/compat/mingw.c b/compat/mingw.c
index abb4d26ce9..38ac35913d 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,5 +1,6 @@
 #include "../git-compat-util.h"
 #include "win32.h"
+#include <aclapi.h>
 #include <conio.h>
 #include <wchar.h>
 #include "../strbuf.h"
@@ -2601,6 +2602,92 @@ static void setup_windows_environment(void)
 	}
 }
 
+static PSID get_current_user_sid(void)
+{
+	HANDLE token;
+	DWORD len = 0;
+	PSID result = NULL;
+
+	if (!OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &token))
+		return NULL;
+
+	if (!GetTokenInformation(token, TokenUser, NULL, 0, &len)) {
+		TOKEN_USER *info = xmalloc((size_t)len);
+		if (GetTokenInformation(token, TokenUser, info, len, &len)) {
+			len = GetLengthSid(info->User.Sid);
+			result = xmalloc(len);
+			if (!CopySid(len, result, info->User.Sid)) {
+				error(_("failed to copy SID (%ld)"),
+				      GetLastError());
+				FREE_AND_NULL(result);
+			}
+		}
+		FREE_AND_NULL(info);
+	}
+	CloseHandle(token);
+
+	return result;
+}
+
+int is_path_owned_by_current_sid(const char *path)
+{
+	WCHAR wpath[MAX_PATH];
+	PSID sid = NULL;
+	PSECURITY_DESCRIPTOR descriptor = NULL;
+	DWORD err;
+
+	static wchar_t home[MAX_PATH];
+
+	int result = 0;
+
+	if (xutftowcs_path(wpath, path) < 0)
+		return 0;
+
+	/*
+	 * On Windows, the home directory is owned by the administrator, but for
+	 * all practical purposes, it belongs to the user. Do pretend that it is
+	 * owned by the user.
+	 */
+	if (!*home) {
+		DWORD size = ARRAY_SIZE(home);
+		DWORD len = GetEnvironmentVariableW(L"HOME", home, size);
+		if (!len || len > size)
+			wcscpy(home, L"::N/A::");
+	}
+	if (!wcsicmp(wpath, home))
+		return 1;
+
+	/* Get the owner SID */
+	err = GetNamedSecurityInfoW(wpath, SE_FILE_OBJECT,
+				    OWNER_SECURITY_INFORMATION |
+				    DACL_SECURITY_INFORMATION,
+				    &sid, NULL, NULL, NULL, &descriptor);
+
+	if (err != ERROR_SUCCESS)
+		error(_("failed to get owner for '%s' (%ld)"), path, err);
+	else if (sid && IsValidSid(sid)) {
+		/* Now, verify that the SID matches the current user's */
+		static PSID current_user_sid;
+
+		if (!current_user_sid)
+			current_user_sid = get_current_user_sid();
+
+		if (current_user_sid &&
+		    IsValidSid(current_user_sid) &&
+		    EqualSid(sid, current_user_sid))
+			result = 1;
+	}
+
+	/*
+	 * We can release the security descriptor struct only now because `sid`
+	 * actually points into this struct.
+	 */
+	if (descriptor)
+		LocalFree(descriptor);
+
+	return result;
+}
+
 int is_valid_win32_path(const char *path, int allow_literal_nul)
 {
 	const char *p = path;
diff --git a/compat/mingw.h b/compat/mingw.h
index af8eddd73e..f6bab548f4 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -452,6 +452,13 @@ char *mingw_query_user_email(void);
 #include <inttypes.h>
 #endif
 
+/**
+ * Verifies that the specified path is owned by the user running the
+ * current process.
+ */
+int is_path_owned_by_current_sid(const char *path);
+#define is_path_owned_by_current_user is_path_owned_by_current_sid
+
 /**
  * Verifies that the given path is a valid one on Windows.
  *
diff --git a/git-compat-util.h b/git-compat-util.h
index 3da9f975e2..63ba89dd31 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -392,6 +392,18 @@ static inline int git_offset_1st_component(const char *path)
 #define is_valid_path(path) 1
 #endif
 
+#ifndef is_path_owned_by_current_user
+static inline int is_path_owned_by_current_uid(const char *path)
+{
+	struct stat st;
+	if (lstat(path, &st))
+		return 0;
+	return st.st_uid == geteuid();
+}
+
+#define is_path_owned_by_current_user is_path_owned_by_current_uid
+#endif
+
 #ifndef find_last_dir_sep
 static inline char *git_find_last_dir_sep(const char *path)
 {
-- 
2.35.1

From 8959555cee7ec045958f9b6dd62e541affb7e7d9 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 2 Mar 2022 12:23:04 +0100
Subject: [PATCH 2/2] 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>
---
 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=<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.
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(&quoted, 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.35.1

Places

File fix-CVE-2022-24765.patch of Package git (Revision 23c92343b85ac3ec9777b356c4c043db)

Places