"Kyle Lippincott via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > From: Kyle Lippincott <spectral@xxxxxxxxxx> > > 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. In other words, if the directory $D that is the top level of the working tree of a non-bare repository, you should be able to chdir to "$D/.git" and run your git command without explicitly saying that you are inside $GIT_DIR (e.g. "git --git-dir=$(pwd) cmd")? It makes very good sense. I briefly wondered if this would give us a great usability improvement especially for hook scripts, but they are given GIT_DIR when called already so that is not a big upside, I guess. > 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. > 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). You are additionally allowing ".git" so even if somebody has ".GIT" that won't be allowed by this change, no? > 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. Yup. If we really cared (which I doubt), we could resort to checking with is_ntfs_dotgit() and is_hfs_dotgit(), but that would work in the direction of loosening the check even further, which I do not know is needed. > - 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; Thanks.
