Merge branch 'defense-in-depth'

This topic branch adds a couple of measures designed to make it much
harder to exploit any bugs in Git's recursive clone machinery that might
be found in the future.

Signed-off-by: Johannes Schindelin <[email protected]>

Author: dscho

Documentation/fsck-msgids.txt

@@ -157,6 +157,18 @@
 `nullSha1`::
 	(WARN) Tree contains entries pointing to a null sha1.
 
+`symlinkPointsToGitDir`::
+	(WARN) Symbolic link points inside a gitdir.
+
+`symlinkTargetBlob`::
+	(ERROR) A non-blob found instead of a symbolic link's target.
+
+`symlinkTargetLength`::
+	(WARN) Symbolic link target longer than maximum path length.
+
+`symlinkTargetMissing`::
+	(ERROR) Unable to read symbolic link target's blob.
+
 `treeNotSorted`::
 	(ERROR) A tree is not properly sorted.
 

builtin/clone.c

@@ -908,6 +908,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	int err = 0, complete_refs_before_fetch = 1;
 	int submodule_progress;
 	int filter_submodules = 0;
+	const char *template_dir;
+	char *template_dir_dup = NULL;
 
 	struct transport_ls_refs_options transport_ls_refs_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -927,6 +929,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		usage_msg_opt(_("You must specify a repository to clone."),
 			builtin_clone_usage, builtin_clone_options);
 
+	xsetenv("GIT_CLONE_PROTECTION_ACTIVE", "true", 0 /* allow user override */);
+	template_dir = get_template_dir(option_template);
+	if (*template_dir && !is_absolute_path(template_dir))
+		template_dir = template_dir_dup =
+			absolute_pathdup(template_dir);
+	xsetenv("GIT_CLONE_TEMPLATE_DIR", template_dir, 1);
+
 	if (option_depth || option_since || option_not.nr)
 		deepen = 1;
 	if (option_single_branch == -1)
@@ -1074,7 +1083,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL,
+	init_db(git_dir, real_git_dir, template_dir, GIT_HASH_UNKNOWN, NULL,
 		INIT_DB_QUIET);
 
 	if (real_git_dir) {
@@ -1392,6 +1401,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	free(unborn_head);
 	free(dir);
 	free(path);
+	free(template_dir_dup);
 	UNLEAK(repo);
 	junk_mode = JUNK_LEAVE_ALL;
 

builtin/init-db.c

@@ -11,10 +11,6 @@
 #include "parse-options.h"
 #include "worktree.h"
 
-#ifndef DEFAULT_GIT_TEMPLATE_DIR
-#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
-#endif
-
 #ifdef NO_TRUSTABLE_FILEMODE
 #define TEST_FILEMODE 0
 #else
@@ -93,8 +89,9 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path,
 	}
 }
 
-static void copy_templates(const char *template_dir, const char *init_template_dir)
+static void copy_templates(const char *option_template)
 {
+	const char *template_dir = get_template_dir(option_template);
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf template_path = STRBUF_INIT;
 	size_t template_len;
@@ -103,16 +100,8 @@ static void copy_templates(const char *template_dir, const char *init_template_d
 	DIR *dir;
 	char *to_free = NULL;
 
-	if (!template_dir)
-		template_dir = getenv(TEMPLATE_DIR_ENVIRONMENT);
-	if (!template_dir)
-		template_dir = init_template_dir;
-	if (!template_dir)
-		template_dir = to_free = system_path(DEFAULT_GIT_TEMPLATE_DIR);
-	if (!template_dir[0]) {
-		free(to_free);
+	if (!template_dir || !*template_dir)
 		return;
-	}
 
 	strbuf_addstr(&template_path, template_dir);
 	strbuf_complete(&template_path, '/');
@@ -200,7 +189,6 @@ static int create_default_files(const char *template_path,
 	int reinit;
 	int filemode;
 	struct strbuf err = STRBUF_INIT;
-	const char *init_template_dir = NULL;
 	const char *work_tree = get_git_work_tree();
 
 	/*
@@ -212,9 +200,7 @@ static int create_default_files(const char *template_path,
 	 * values (since we've just potentially changed what's available on
 	 * disk).
 	 */
-	git_config_get_pathname("init.templatedir", &init_template_dir);
-	copy_templates(template_path, init_template_dir);
-	free((char *)init_template_dir);
+	copy_templates(template_path);
 	git_config_clear();
 	reset_shared_repository();
 	git_config(git_default_config, NULL);

cache.h

@@ -656,6 +656,7 @@ int path_inside_repo(const char *prefix, const char *path);
 #define INIT_DB_QUIET 0x0001
 #define INIT_DB_EXIST_OK 0x0002
 
+const char *get_template_dir(const char *option_template);
 int init_db(const char *git_dir, const char *real_git_dir,
 	    const char *template_dir, int hash_algo,
 	    const char *initial_branch, unsigned int flags);
@@ -1784,6 +1785,20 @@ int copy_fd(int ifd, int ofd);
 int copy_file(const char *dst, const char *src, int mode);
 int copy_file_with_time(const char *dst, const char *src, int mode);
 
+/*
+ * Compare the file mode and contents of two given files.
+ *
+ * If both files are actually symbolic links, the function returns 1 if the link
+ * targets are identical or 0 if they are not.
+ *
+ * If any of the two files cannot be accessed or in case of read failures, this
+ * function returns 0.
+ *
+ * If the file modes and contents are identical, the function returns 1,
+ * otherwise it returns 0.
+ */
+int do_files_match(const char *path1, const char *path2);
+
 void write_or_die(int fd, const void *buf, size_t count);
 void fsync_or_die(int fd, const char *);
 int fsync_component(enum fsync_component component, int fd);

config.c

@@ -1525,8 +1525,19 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
-	if (!strcmp(var, "core.hookspath"))
+	if (!strcmp(var, "core.hookspath")) {
+		if (current_config_scope() == CONFIG_SCOPE_LOCAL &&
+		    git_env_bool("GIT_CLONE_PROTECTION_ACTIVE", 0))
+			die(_("active `core.hooksPath` found in the local "
+			      "repository config:\n\t%s\nFor security "
+			      "reasons, this is disallowed by default.\nIf "
+			      "this is intentional and the hook should "
+			      "actually be run, please\nrun the command "
+			      "again with "
+			      "`GIT_CLONE_PROTECTION_ACTIVE=false`"),
+			    value);
 		return git_config_pathname(&git_hooks_path, var, value);
+	}
 
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);

copy.c

@@ -65,3 +65,61 @@ int copy_file_with_time(const char *dst, const char *src, int mode)
 		return copy_times(dst, src);
 	return status;
 }
+
+static int do_symlinks_match(const char *path1, const char *path2)
+{
+	struct strbuf buf1 = STRBUF_INIT, buf2 = STRBUF_INIT;
+	int ret = 0;
+
+	if (!strbuf_readlink(&buf1, path1, 0) &&
+	    !strbuf_readlink(&buf2, path2, 0))
+		ret = !strcmp(buf1.buf, buf2.buf);
+
+	strbuf_release(&buf1);
+	strbuf_release(&buf2);
+	return ret;
+}
+
+int do_files_match(const char *path1, const char *path2)
+{
+	struct stat st1, st2;
+	int fd1 = -1, fd2 = -1, ret = 1;
+	char buf1[8192], buf2[8192];
+
+	if ((fd1 = open_nofollow(path1, O_RDONLY)) < 0 ||
+	    fstat(fd1, &st1) || !S_ISREG(st1.st_mode)) {
+		if (fd1 < 0 && errno == ELOOP)
+			/* maybe this is a symbolic link? */
+			return do_symlinks_match(path1, path2);
+		ret = 0;
+	} else if ((fd2 = open_nofollow(path2, O_RDONLY)) < 0 ||
+		   fstat(fd2, &st2) || !S_ISREG(st2.st_mode)) {
+		ret = 0;
+	}
+
+	if (ret)
+		/* to match, neither must be executable, or both */
+		ret = !(st1.st_mode & 0111) == !(st2.st_mode & 0111);
+
+	if (ret)
+		ret = st1.st_size == st2.st_size;
+
+	while (ret) {
+		ssize_t len1 = read_in_full(fd1, buf1, sizeof(buf1));
+		ssize_t len2 = read_in_full(fd2, buf2, sizeof(buf2));
+
+		if (len1 < 0 || len2 < 0 || len1 != len2)
+			ret = 0; /* read error or different file size */
+		else if (!len1) /* len2 is also 0; hit EOF on both */
+			break; /* ret is still true */
+		else
+			ret = !memcmp(buf1, buf2, len1);
+	}
+
+	if (fd1 >= 0)
+		close(fd1);
+	if (fd2 >= 0)
+		close(fd2);
+
+	return ret;
+}

dir.c

@@ -88,6 +88,18 @@ int fspathncmp(const char *a, const char *b, size_t count)
 	return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
 }
 
+int paths_collide(const char *a, const char *b)
+{
+	size_t len_a = strlen(a), len_b = strlen(b);
+
+	if (len_a == len_b)
+		return fspatheq(a, b);
+
+	if (len_a < len_b)
+		return is_dir_sep(b[len_a]) && !fspathncmp(a, b, len_a);
+	return is_dir_sep(a[len_b]) && !fspathncmp(a, b, len_b);
+}
+
 unsigned int fspathhash(const char *str)
 {
 	return ignore_case ? strihash(str) : strhash(str);

dir.h

@@ -519,6 +519,13 @@ int fspatheq(const char *a, const char *b);
 int fspathncmp(const char *a, const char *b, size_t count);
 unsigned int fspathhash(const char *str);
 
+/*
+ * Reports whether paths collide. This may be because the paths differ only in
+ * case on a case-sensitive filesystem, or that one path refers to a symlink
+ * that collides with one of the parent directories of the other.
+ */
+int paths_collide(const char *a, const char *b);
+
 /*
  * The prefix part of pattern must not contains wildcards.
  */

entry.c

@@ -454,7 +454,7 @@ static void mark_colliding_entries(const struct checkout *state,
 			continue;
 
 		if ((trust_ino && !match_stat_data(&dup->ce_stat_data, st)) ||
-		    (!trust_ino && !fspathcmp(ce->name, dup->name))) {
+		    paths_collide(ce->name, dup->name)) {
 			dup->ce_flags |= CE_MATCHED;
 			break;
 		}
@@ -541,6 +541,20 @@ int checkout_entry_ca(struct cache_entry *ce, struct conv_attrs *ca,
 			/* If it is a gitlink, leave it alone! */
 			if (S_ISGITLINK(ce->ce_mode))
 				return 0;
+			/*
+			 * We must avoid replacing submodules' leading
+			 * directories with symbolic links, lest recursive
+			 * clones can write into arbitrary locations.
+			 *
+			 * Technically, this logic is not limited
+			 * to recursive clones, or for that matter to
+			 * submodules' paths colliding with symbolic links'
+			 * paths. Yet it strikes a balance in favor of
+			 * simplicity, and if paths are colliding, we might
+			 * just as well keep the directories during a clone.
+			 */
+			if (state->clone && S_ISLNK(ce->ce_mode))
+				return 0;
 			remove_subtree(&path);
 		} else if (unlink(path.buf))
 			return error_errno("unable to unlink old '%s'", path.buf);

fsck.c

@@ -636,6 +636,8 @@ static int fsck_tree(const struct object_id *tree_oid,
 				retval += report(options, tree_oid, OBJ_TREE,
 						 FSCK_MSG_MAILMAP_SYMLINK,
 						 ".mailmap is a symlink");
+			oidset_insert(&options->symlink_targets_found,
+				      entry_oid);
 		}
 
 		if ((backslash = strchr(name, '\\'))) {
@@ -1228,6 +1230,56 @@ static int fsck_blob(const struct object_id *oid, const char *buf,
 		}
 	}
 
+	if (oidset_contains(&options->symlink_targets_found, oid)) {
+		const char *ptr = buf;
+		const struct object_id *reported = NULL;
+
+		oidset_insert(&options->symlink_targets_done, oid);
+
+		if (!buf || size > PATH_MAX) {
+			/*
+			 * A missing buffer here is a sign that the caller found the
+			 * blob too gigantic to load into memory. Let's just consider
+			 * that an error.
+			 */
+			return report(options, oid, OBJ_BLOB,
+					FSCK_MSG_SYMLINK_TARGET_LENGTH,
+					"symlink target too long");
+		}
+
+		while (!reported && ptr) {
+			const char *p = ptr;
+			char c, *slash = strchrnul(ptr, '/');
+			char *backslash = memchr(ptr, '\\', slash - ptr);
+
+			c = *slash;
+			*slash = '\0';
+
+			while (!reported && backslash) {
+				*backslash = '\0';
+				if (is_ntfs_dotgit(p))
+					ret |= report(options, reported = oid, OBJ_BLOB,
+						      FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
+						      "symlink target points to git dir");
+				*backslash = '\\';
+				p = backslash + 1;
+				backslash = memchr(p, '\\', slash - p);
+			}
+			if (!reported && is_ntfs_dotgit(p))
+				ret |= report(options, reported = oid, OBJ_BLOB,
+					      FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
+					      "symlink target points to git dir");
+
+			if (!reported && is_hfs_dotgit(ptr))
+				ret |= report(options, reported = oid, OBJ_BLOB,
+					      FSCK_MSG_SYMLINK_POINTS_TO_GIT_DIR,
+					      "symlink target points to git dir");
+
+			*slash = c;
+			ptr = c ? slash + 1 : NULL;
+		}
+	}
+
 	return ret;
 }
 
@@ -1319,6 +1371,10 @@ int fsck_finish(struct fsck_options *options)
 			  FSCK_MSG_GITATTRIBUTES_MISSING, FSCK_MSG_GITATTRIBUTES_BLOB,
 			  options, ".gitattributes");
 
+	ret |= fsck_blobs(&options->symlink_targets_found, &options->symlink_targets_done,
+			  FSCK_MSG_SYMLINK_TARGET_MISSING, FSCK_MSG_SYMLINK_TARGET_BLOB,
+			  options, "<symlink-target>");
+
 	return ret;
 }