[PATCH] setup: allow cwd=.git w/ bareRepository=explicit

From: Kyle Lippincott via GitGitGadget <gitgitgadget@gmail.com>
Date: Jan 20, 2024, 00:08 UTC
Message-ID: <pull.1645.git.1705709303098.gitgitgadget@gmail.com>

From: Kyle Lippincott <spectral@google.com>

The safe.bareRepository setting can be set to 'explicit' to disallow implicit uses of bare repositories, preventing an attack [1] where an artificial and malicious bare repository is embedded in another git repository. Unfortunately, some tooling uses myrepo/.git/ as the cwd when executing commands, and this is blocked when safe.bareRepository=explicit. Blocking is unnecessary, as git already prevents nested .git directories.

Teach git to not reject uses of git inside of the .git directory: check if cwd is .git (or a subdirectory of it) and allow it even if safe.bareRepository=explicit.

[1] https://github.com/justinsteven/advisories/blob/main/2022_git_buried_bare_repos_and_fsmonitor_various_abuses.md

Signed-off-by: Kyle Lippincott <spectral@google.com>
---
    setup: allow cwd=.git w/ bareRepository=explicit
    
    Please be aware that I'm a new contributor (this is my first patch to
    git's code), so any style nits, suggestions about how to make this more
    idiomatic, or any other suggestions are strongly encouraged.
    
    My primary concern with this patch is that I'm unsure if we need to
    worry about case-insensitive filesystems (ex: cwd=my_repo/.GIT instead
    of my_repo/.git, it might not trigger this logic and end up allowed).
    I'm assuming this isn't a significant concern, for two reasons:
    
     * most filesystems/OSes in use today (by number of users) are at least
       case-preserving, so users/tools will have had to type out .GIT
       instead of getting it from readdir/wherever.
     * this is primarily a "quality of life" change to the feature, and if
       we get it wrong we still fail closed.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1645%2Fspectral54%2Fbare-repo-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1645/spectral54/bare-repo-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1645

 setup.c                         | 3 ++-
 t/t0035-safe-bare-repository.sh | 8 ++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index b38702718fb..b095e284979 100644
--- a/setup.c
+++ b/setup.c
@@ -1371,7 +1371,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 
 		if (is_git_directory(dir->buf)) {
 			trace2_data_string("setup", NULL, "implicit-bare-repository", dir->buf);
-			if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT)
+			if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT &&
+			    !ends_with_path_components(dir->buf, ".git"))
 				return GIT_DIR_DISALLOWED_BARE;
 			if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
 				return GIT_DIR_INVALID_OWNERSHIP;
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index 038b8b788d7..80488563795 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -78,4 +78,12 @@ test_expect_success 'no trace when GIT_DIR is explicitly provided' '
 	expect_accepted_explicit "$pwd/outer-repo/bare-repo"
 '
 
+test_expect_success 'no trace when "bare repository" is .git' '
+	expect_accepted_implicit -C outer-repo/.git
+'
+
+test_expect_success 'no trace when "bare repository" is a subdir of .git' '
+	expect_accepted_implicit -C outer-repo/.git/objects
+'
+
 test_done

base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5

-- 
gitgitgadget