From 10ecfa76491e4923988337b2e2243b05376b40de Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Fri, 4 May 2018 20:03:35 -0400
Subject: [PATCH 10/11] verify_path: disallow symlinks in .gitmodules

There are a few reasons it's not a good idea to make
.gitmodules a symlink, including:

  1. It won't be portable to systems without symlinks.

  2. It may behave inconsistently, since Git may look at
     this file in the index or a tree without bothering to
     resolve any symbolic links. We don't do this _yet_, but
     the config infrastructure is there and it's planned for
     the future.

With some clever code, we could make (2) work. And some
people may not care about (1) if they only work on one
platform. But there are a few security reasons to simply
disallow it:

  a. A symlinked .gitmodules file may circumvent any fsck
     checks of the content.

  b. Git may read and write from the on-disk file without
     sanity checking the symlink target. So for example, if
     you link ".gitmodules" to "../oops" and run "git
     submodule add", we'll write to the file "oops" outside
     the repository.

Again, both of those are problems that _could_ be solved
with sufficient code, but given the complications in (1) and
(2), we're better off just outlawing it explicitly.

Note the slightly tricky call to verify_path() in
update-index's update_one(). There we may not have a mode if
we're not updating from the filesystem (e.g., we might just
be removing the file). Passing "0" as the mode there works
fine; since it's not a symlink, we'll just skip the extra
checks.

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c                |    4 ++--
 builtin/update-index.c |    6 +++---
 cache.h                |    2 +-
 read-cache.c           |   40 +++++++++++++++++++++++++++++++---------
 4 files changed, 37 insertions(+), 15 deletions(-)

Index: git-1.7.12.4/builtin/update-index.c
===================================================================
--- git-1.7.12.4.orig/builtin/update-index.c
+++ git-1.7.12.4/builtin/update-index.c
@@ -220,7 +220,7 @@ static int add_cacheinfo(unsigned int mo
 	int size, len, option;
 	struct cache_entry *ce;
 
-	if (!verify_path(path))
+	if (!verify_path(path, mode))
 		return error("Invalid path '%s'", path);
 
 	len = strlen(path);
@@ -285,7 +285,7 @@ static void update_one(const char *path,
 	} /* else stat is valid */
 
 	const char *p = prefix_path(prefix, prefix_length, path);
-	if (!verify_path(p)) {
+	if (!verify_path(p, st.st_mode)) {
 		fprintf(stderr, "Ignoring path %s\n", path);
 		goto free_return;
 	}
@@ -376,7 +376,7 @@ static void read_index_info(int line_ter
 			path_name = uq.buf;
 		}
 
-		if (!verify_path(path_name)) {
+		if (!verify_path(path_name, mode)) {
 			fprintf(stderr, "Ignoring path %s\n", path_name);
 			continue;
 		}
Index: git-1.7.12.4/cache.h
===================================================================
--- git-1.7.12.4.orig/cache.h
+++ git-1.7.12.4/cache.h
@@ -440,7 +440,7 @@ extern int read_index_unmerged(struct in
 extern int write_index(struct index_state *, int newfd);
 extern int discard_index(struct index_state *);
 extern int unmerged_index(const struct index_state *);
-extern int verify_path(const char *path);
+extern int verify_path(const char *path, unsigned mode);
 extern struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int igncase);
 extern int index_name_stage_pos(const struct index_state *, const char *name, int namelen, int stage);
 extern int index_name_pos(const struct index_state *, const char *name, int namelen);
Index: git-1.7.12.4/read-cache.c
===================================================================
--- git-1.7.12.4.orig/read-cache.c
+++ git-1.7.12.4/read-cache.c
@@ -681,7 +681,7 @@ struct cache_entry *make_cache_entry(uns
 	int size, len;
 	struct cache_entry *ce;
 
-	if (!verify_path(path)) {
+	if (!verify_path(path, mode)) {
 		error("Invalid path '%s'", path);
 		return NULL;
 	}
@@ -721,7 +721,7 @@ int ce_path_match(const struct cache_ent
  * Also, we don't want double slashes or slashes at the
  * end that can make pathnames ambiguous.
  */
-static int verify_dotfile(const char *rest)
+static int verify_dotfile(const char *rest, unsigned mode)
 {
 	/*
 	 * The first character was '.', but that
@@ -739,6 +739,9 @@ static int verify_dotfile(const char *re
 	 * case-insensitively here, even if ignore_case is not set.
 	 * This outlaws ".GIT" everywhere out of an abundance of caution,
 	 * since there's really no good reason to allow it.
+	 *
+	 * Once we've seen ".git", we can also find ".gitmodules", etc (also
+	 * case-insensitively).
 	 */
 	case 'g':
 	case 'G':
@@ -748,6 +751,12 @@ static int verify_dotfile(const char *re
 			break;
 		if (rest[3] == '\0' || is_dir_sep(rest[3]))
 			return 0;
+		if (S_ISLNK(mode)) {
+			rest += 3;
+			if (skip_iprefix(rest, "modules", &rest) &&
+			    (*rest == '\0' || is_dir_sep(*rest)))
+				return 0;
+		}
 		break;
 	case '.':
 		if (rest[1] == '\0' || is_dir_sep(rest[1]))
@@ -756,7 +765,7 @@ static int verify_dotfile(const char *re
 	return 1;
 }
 
-int verify_path(const char *path)
+int verify_path(const char *path, unsigned mode)
 {
 	char c;
 
@@ -769,12 +778,25 @@ int verify_path(const char *path)
 			return 1;
 		if (is_dir_sep(c)) {
 inside:
-			if (protect_hfs && is_hfs_dotgit(path))
-				return 0;
-			if (protect_ntfs && is_ntfs_dotgit(path))
-				return 0;
+			if (protect_hfs) {
+				if (is_hfs_dotgit(path))
+					return 0;
+				if (S_ISLNK(mode)) {
+					if (is_hfs_dotgitmodules(path))
+						return 0;
+				}
+			}
+			if (protect_ntfs) {
+				if (is_ntfs_dotgit(path))
+					return 0;
+				if (S_ISLNK(mode)) {
+					if (is_ntfs_dotgitmodules(path))
+						return 0;
+				}
+			}
+
 			c = *path++;
-			if ((c == '.' && !verify_dotfile(path)) ||
+			if ((c == '.' && !verify_dotfile(path, mode)) ||
 			    is_dir_sep(c) || c == '\0')
 				return 0;
 		}
@@ -952,7 +974,7 @@ static int add_index_entry_with_check(st
 
 	if (!ok_to_add)
 		return -1;
-	if (!verify_path(ce->name))
+	if (!verify_path(ce->name, ce->ce_mode))
 		return error("Invalid path '%s'", ce->name);
 
 	if (!skip_df_check &&
