[PATCH/RFC] fsck: complain when .gitignore and .gitattributes are symlinks

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 14, 2019, 23:09 UTC
Message-ID: <20190114230902.GG162110@google.com>

From: Jeff King <peff@peff.net>
Date: Sun, 13 May 2018 14:14:34 -0400

This case is already forbidden by verify_path(), so let's check it in fsck. It's easier to handle than .gitmodules, because we don't care about checking the blob content. This is really just about whether the name and mode for the tree entry are valid.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,

This patch is from the 2.20.0 era, from the same series as

 fsck: detect submodule urls starting with dash

It was omitted from that series because it does not address any known exploit, but to me it seems worthwhile anyway:

- if a client enables transfer.fsckObjects, this helps them protect
  themselves against weird input that does *not* have a known exploit
  attached, to

- it generally feels more simple and robust.  Git-related tools can
  benefit from this kind of check as an indication of input they can
  bail out on instead of trying to support.

Peff checked it against repos in the wild and found this to be very rare but existent (e.g. https://github.com/acquia/blt has a .gitattributes symlink). Linus suggested that we may want it to be INFO instead of ERROR, so that people can at least notice that their .gitattributes symlink is likely to have no effect. This patch still uses ERROR because I suspect that this is rare enough in the wild that people will be able to cope.

Thoughts?

Thanks, Jonathan

 fsck.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fsck.c b/fsck.c
index 68502ce85b..850363fc8e 100644
--- a/fsck.c
+++ b/fsck.c
@@ -68,6 +68,8 @@ static struct oidset gitmodules_done = OIDSET_INIT;
 	FUNC(GITMODULES_SYMLINK, ERROR) \
 	FUNC(GITMODULES_URL, ERROR) \
 	FUNC(GITMODULES_PATH, ERROR) \
+	FUNC(GITIGNORE_SYMLINK, ERROR) \
+	FUNC(GITATTRIBUTES_SYMLINK, ERROR) \
 	/* warnings */ \
 	FUNC(BAD_FILEMODE, WARN) \
 	FUNC(EMPTY_NAME, WARN) \
@@ -627,6 +629,19 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
 						 ".gitmodules is a symbolic link");
 		}
 
+		if (S_ISLNK(mode)) {
+			if (is_hfs_dotgitignore(name) ||
+			    is_ntfs_dotgitignore(name))
+				retval += report(options, &item->object,
+						 FSCK_MSG_GITIGNORE_SYMLINK,
+						 ".gitignore is a symlink");
+			if (is_hfs_dotgitattributes(name) ||
+			    is_ntfs_dotgitattributes(name))
+				retval += report(options, &item->object,
+						 FSCK_MSG_GITATTRIBUTES_SYMLINK,
+						 ".gitattributes is a symlink");
+		}
+
 		if (update_tree_entry_gently(&desc)) {
 			retval += report(options, &item->object, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree");
 			break;

-- 
2.20.1.97.g81188d93c3