Hi Glen, On Wed, 6 Apr 2022, Glen Choo wrote: > Git tries not to distribute configs in-repo because they are a security > risk. However, an attacker can do exactly this if they embed a bare > repo inside of another repo. > > Teach fsck to detect whether a tree object contains a bare repo (as > determined by setup.c) and warn. This will help hosting sites detect and > prevent transmission of such malicious repos. > > See [1] for a more in-depth discussion, including future steps and > alternatives. > > [1] https://lore.kernel.org/git/kl6lsfqpygsj.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ Out of curiosity: does this new check trigger with https://github.com/libgit2/libgit2? AFAIR it has embedded repositories that are used in its test suite. In other words, libgit2 has a legitimate use case for embedded bare repositories, I believe. Thank you, Johannes > > Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx> > --- > fsck.c | 19 +++++++++++++++++++ > fsck.h | 1 + > setup.c | 4 ++++ > t/t1450-fsck.sh | 36 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 60 insertions(+) > > diff --git a/fsck.c b/fsck.c > index 3ec500d707..11c11c348a 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -573,6 +573,9 @@ static int fsck_tree(const struct object_id *tree_oid, > int has_bad_modes = 0; > int has_dup_entries = 0; > int not_properly_sorted = 0; > + int has_head = 0; > + int has_refs_entry = 0; > + int has_objects_entry = 0; > struct tree_desc desc; > unsigned o_mode; > const char *o_name; > @@ -602,6 +605,12 @@ static int fsck_tree(const struct object_id *tree_oid, > has_dotdot |= !strcmp(name, ".."); > has_dotgit |= is_hfs_dotgit(name) || is_ntfs_dotgit(name); > has_zero_pad |= *(char *)desc.buffer == '0'; > + has_head |= !strcasecmp(name, "HEAD") > + && (S_ISLNK(mode) || S_ISREG(mode)); > + has_refs_entry |= !strcasecmp(name, "refs") > + && (S_ISLNK(mode) || S_ISDIR(mode)); > + has_objects_entry |= !strcasecmp(name, "objects") > + && (S_ISLNK(mode) || S_ISDIR(mode)); > > if (is_hfs_dotgitmodules(name) || is_ntfs_dotgitmodules(name)) { > if (!S_ISLNK(mode)) > @@ -739,6 +748,16 @@ static int fsck_tree(const struct object_id *tree_oid, > retval += report(options, tree_oid, OBJ_TREE, > FSCK_MSG_TREE_NOT_SORTED, > "not properly sorted"); > + /* > + * Determine if this tree looks like a bare repository according > + * to the rules of setup.c. If those are changed, this should be > + * changed too. > + */ > + if (has_head && has_refs_entry && has_objects_entry) > + retval += report(options, tree_oid, OBJ_TREE, > + FSCK_MSG_EMBEDDED_BARE_REPO, > + "contains bare repository"); > + > return retval; > } > > diff --git a/fsck.h b/fsck.h > index d07f7a2459..3f0f73b0f3 100644 > --- a/fsck.h > +++ b/fsck.h > @@ -65,6 +65,7 @@ enum fsck_msg_type { > FUNC(NULL_SHA1, WARN) \ > FUNC(ZERO_PADDED_FILEMODE, WARN) \ > FUNC(NUL_IN_COMMIT, WARN) \ > + FUNC(EMBEDDED_BARE_REPO, WARN) \ > /* infos (reported as warnings, but ignored by default) */ \ > FUNC(GITMODULES_PARSE, INFO) \ > FUNC(GITIGNORE_SYMLINK, INFO) \ > diff --git a/setup.c b/setup.c > index 04ce33cdcd..2600548776 100644 > --- a/setup.c > +++ b/setup.c > @@ -336,6 +336,10 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir) > * - either a HEAD symlink or a HEAD file that is formatted as > * a proper "ref:", or a regular file HEAD that has a properly > * formatted sha1 object name. > + * > + * fsck.c checks for bare repositories in trees using similar rules, but a > + * duplicated implementation. If these are changed, the correspnding code in > + * fsck.c should change too. > */ > int is_git_directory(const char *suspect) > { > diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh > index de50c0ea01..a65827bc03 100755 > --- a/t/t1450-fsck.sh > +++ b/t/t1450-fsck.sh > @@ -563,6 +563,42 @@ dot-backslash-case .\\\\.GIT\\\\foobar > dotgit-case-backslash .git\\\\foobar > EOF > > +test_expect_success "fsck notices bare repo" ' > +( > + mkdir -p embedded-bare-repo/bare && > + git init embedded-bare-repo && > + ( > + cd embedded-bare-repo/bare && > + echo content >HEAD && > + mkdir refs/ objects/ && > + echo content >refs/foo && > + echo content >objects/foo && > + git add . && > + git commit -m base && > + bad_tree=$(git rev-parse HEAD:bare) && > + git fsck 2>out && > + test_i18ngrep "warning.*tree $bad_tree: embeddedBareRepo: contains bare repository" out > + ) > +)' > + > +test_expect_success "fsck notices bare repo with odd casing" ' > +( > + mkdir -p embedded-bare-repo-case/bare && > + git init embedded-bare-repo-case && > + ( > + cd embedded-bare-repo-case/bare && > + echo content >heAD && > + mkdir Refs/ objectS/ && > + echo content >Refs/foo && > + echo content >objectS/foo && > + git add . && > + git commit -m base && > + bad_tree=$(git rev-parse HEAD:bare) && > + git fsck 2>out && > + test_i18ngrep "warning.*tree $bad_tree: embeddedBareRepo: contains bare repository" out > + ) > +)' > + > test_expect_success 'fsck allows .??it' ' > ( > git init not-dotgit && > > base-commit: 805e0a68082a217f0112db9ee86a022227a9c81b > -- > 2.33.GIT > >
