verify_path: disallow symlinks in .gitmodules, etc

There are a few reasons it's not a good idea to make
.git* files symlinks, including:

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

  2. It may behave inconsistently, since Git internally may
     look at these files from the index or a tree without
     bothering to resolve any symbolic links. So it may work
     in some settings (where we read from the filesystem)
     but not in others).

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 symlinked meta-files:

  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 current inconsistent
behavior and unportability, we're better off just outlawing
it explicitly.

We'll give the same treatment to .gitmodules, .gitignore,
and .gitattributes. The latter two cannot be used to write
outside the repository (we write them only as part of a
checkout, where we are careful not to follow any symlinks).
But they can still cause a "git clone && git log"
combination to read arbitrary files outside the filesystem.
There's _probably_ nothing too harmful you can do with that,
but it seems questionable (and anyway, they suffer from the
same portability and consistency problems).

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 <[email protected]>

Author: peff

apply.c

@@ -3867,9 +3867,9 @@ static int check_unsafe_path(struct patch *patch)
 	if (!patch->is_delete)
 		new_name = patch->new_name;
 
-	if (old_name && !verify_path(old_name))
+	if (old_name && !verify_path(old_name, patch->old_mode))
 		return error(_("invalid path '%s'"), old_name);
-	if (new_name && !verify_path(new_name))
+	if (new_name && !verify_path(new_name, patch->new_mode))
 		return error(_("invalid path '%s'"), new_name);
 	return 0;
 }

builtin/update-index.c

@@ -399,7 +399,7 @@ static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
 	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);
@@ -452,7 +452,7 @@ static void update_one(const char *path)
 		stat_errno = errno;
 	} /* else stat is valid */
 
-	if (!verify_path(path)) {
+	if (!verify_path(path, st.st_mode)) {
 		fprintf(stderr, "Ignoring path %s\n", path);
 		return;
 	}
@@ -543,7 +543,7 @@ static void read_index_info(int nul_term_line)
 			path_name = uq.buf;
 		}
 
-		if (!verify_path(path_name)) {
+		if (!verify_path(path_name, mode)) {
 			fprintf(stderr, "Ignoring path %s\n", path_name);
 			continue;
 		}

cache.h

@@ -598,7 +598,7 @@ extern int read_index_unmerged(struct index_state *);
 extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
 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 int strcmp_offset(const char *s1, const char *s2, size_t *first_change);
 extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
 extern void adjust_dirname_case(struct index_state *istate, char *name);

read-cache.c

@@ -732,7 +732,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
 	int size, len;
 	struct cache_entry *ce, *ret;
 
-	if (!verify_path(path)) {
+	if (!verify_path(path, mode)) {
 		error("Invalid path '%s'", path);
 		return NULL;
 	}
@@ -796,7 +796,7 @@ int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
  * 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
@@ -814,6 +814,9 @@ static int verify_dotfile(const char *rest)
 	 * 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':
@@ -823,6 +826,14 @@ static int verify_dotfile(const char *rest)
 			break;
 		if (rest[3] == '\0' || is_dir_sep(rest[3]))
 			return 0;
+		if (S_ISLNK(mode)) {
+			rest += 3;
+			if ((skip_iprefix(rest, "modules", &rest) ||
+			     skip_iprefix(rest, "ignore", &rest) ||
+			     skip_iprefix(rest, "attributes", &rest)) &&
+			    (*rest == '\0' || is_dir_sep(*rest)))
+				return 0;
+		}
 		break;
 	case '.':
 		if (rest[1] == '\0' || is_dir_sep(rest[1]))
@@ -831,7 +842,7 @@ static int verify_dotfile(const char *rest)
 	return 1;
 }
 
-int verify_path(const char *path)
+int verify_path(const char *path, unsigned mode)
 {
 	char c;
 
@@ -844,12 +855,29 @@ 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) ||
+					    is_hfs_dotgitignore(path) ||
+					    is_hfs_dotgitattributes(path))
+						return 0;
+				}
+			}
+			if (protect_ntfs) {
+				if (is_ntfs_dotgit(path))
+					return 0;
+				if (S_ISLNK(mode)) {
+					if (is_ntfs_dotgitmodules(path) ||
+					    is_ntfs_dotgitignore(path) ||
+					    is_ntfs_dotgitattributes(path))
+						return 0;
+				}
+			}
+
 			c = *path++;
-			if ((c == '.' && !verify_dotfile(path)) ||
+			if ((c == '.' && !verify_dotfile(path, mode)) ||
 			    is_dir_sep(c) || c == '\0')
 				return 0;
 		}
@@ -1166,7 +1194,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e
 
 	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 &&