Merge pull request #3619 from ethomson/win32_forbidden

win32: allow us to read indexes with forbidden paths on win32

src/checkout.c

@@ -1226,7 +1226,7 @@ static int checkout_verify_paths(
 	int action,
 	git_diff_delta *delta)
 {
-	unsigned int flags = GIT_PATH_REJECT_DEFAULTS | GIT_PATH_REJECT_DOT_GIT;
+	unsigned int flags = GIT_PATH_REJECT_WORKDIR_DEFAULTS;
 
 	if (action & CHECKOUT_ACTION__REMOVE) {
 		if (!git_path_isvalid(repo, delta->old_file.path, flags)) {

src/index.c

@@ -853,17 +853,31 @@ static void index_entry_adjust_namemask(
 		entry->flags |= GIT_IDXENTRY_NAMEMASK;
 }
 
+/* When `from_workdir` is true, we will validate the paths to avoid placing
+ * paths that are invalid for the working directory on the current filesystem
+ * (eg, on Windows, we will disallow `GIT~1`, `AUX`, `COM1`, etc).  This
+ * function will *always* prevent `.git` and directory traversal `../` from
+ * being added to the index.
+ */
 static int index_entry_create(
 	git_index_entry **out,
 	git_repository *repo,
-	const char *path)
+	const char *path,
+	bool from_workdir)
 {
 	size_t pathlen = strlen(path), alloclen;
 	struct entry_internal *entry;
+	unsigned int path_valid_flags = GIT_PATH_REJECT_INDEX_DEFAULTS;
 
-	if (!git_path_isvalid(repo, path,
-		GIT_PATH_REJECT_DEFAULTS | GIT_PATH_REJECT_DOT_GIT)) {
-		giterr_set(GITERR_INDEX, "Invalid path: '%s'", path);
+	/* always reject placing `.git` in the index and directory traversal.
+	 * when requested, disallow platform-specific filenames and upgrade to
+	 * the platform-specific `.git` tests (eg, `git~1`, etc).
+	 */
+	if (from_workdir)
+		path_valid_flags |= GIT_PATH_REJECT_WORKDIR_DEFAULTS;
+
+	if (!git_path_isvalid(repo, path, path_valid_flags)) {
+		giterr_set(GITERR_INDEX, "invalid path: '%s'", path);
 		return -1;
 	}
 
@@ -895,7 +909,7 @@ static int index_entry_init(
 			"Could not initialize index entry. "
 			"Index is not backed up by an existing repository.");
 
-	if (index_entry_create(&entry, INDEX_OWNER(index), rel_path) < 0)
+	if (index_entry_create(&entry, INDEX_OWNER(index), rel_path, true) < 0)
 		return -1;
 
 	/* write the blob to disk and get the oid and stat info */
@@ -975,7 +989,7 @@ static int index_entry_dup(
 	git_index *index,
 	const git_index_entry *src)
 {
-	if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0)
+	if (index_entry_create(out, INDEX_OWNER(index), src->path, false) < 0)
 		return -1;
 
 	index_entry_cpy(*out, src);
@@ -997,7 +1011,7 @@ static int index_entry_dup_nocache(
 	git_index *index,
 	const git_index_entry *src)
 {
-	if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0)
+	if (index_entry_create(out, INDEX_OWNER(index), src->path, false) < 0)
 		return -1;
 
 	index_entry_cpy_nocache(*out, src);
@@ -1402,7 +1416,7 @@ static int add_repo_as_submodule(git_index_entry **out, git_index *index, const
 	struct stat st;
 	int error;
 
-	if (index_entry_create(&entry, INDEX_OWNER(index), path) < 0)
+	if (index_entry_create(&entry, INDEX_OWNER(index), path, true) < 0)
 		return -1;
 
 	if ((error = git_buf_joinpath(&abspath, git_repository_workdir(repo), path)) < 0)
@@ -2788,7 +2802,7 @@ static int read_tree_cb(
 	if (git_buf_joinpath(&path, root, tentry->filename) < 0)
 		return -1;
 
-	if (index_entry_create(&entry, INDEX_OWNER(data->index), path.ptr) < 0)
+	if (index_entry_create(&entry, INDEX_OWNER(data->index), path.ptr, false) < 0)
 		return -1;
 
 	entry->mode = tentry->attr;

src/iterator.c

@@ -558,6 +558,8 @@ static bool tree_iterator__pop_frame(tree_iterator *ti, bool final)
 {
 	tree_iterator_frame *tf = ti->head;
 
+	assert(tf);
+
 	if (!tf->up)
 		return false;
 
@@ -581,6 +583,8 @@ static void tree_iterator__pop_all(tree_iterator *ti, bool to_end, bool final)
 	while (tree_iterator__pop_frame(ti, final)) /* pop to root */;
 
 	if (!final) {
+		assert(ti->head);
+
 		ti->head->current = to_end ? ti->head->n_entries : 0;
 		ti->path_ambiguities = 0;
 		git_buf_clear(&ti->path);
@@ -773,10 +777,12 @@ static void tree_iterator__free(git_iterator *self)
 {
 	tree_iterator *ti = (tree_iterator *)self;
 
-	tree_iterator__pop_all(ti, true, false);
+	if (ti->head) {
+		tree_iterator__pop_all(ti, true, false);
+		git_tree_free(ti->head->entries[0]->tree);
+		git__free(ti->head);
+	}
 
-	git_tree_free(ti->head->entries[0]->tree);
-	git__free(ti->head);
 	git_pool_clear(&ti->pool);
 	git_buf_free(&ti->path);
 }

src/path.c

@@ -1630,9 +1630,12 @@ static bool verify_component(
 		!verify_dotgit_ntfs(repo, component, len))
 		return false;
 
+	/* don't bother rerunning the `.git` test if we ran the HFS or NTFS
+	 * specific tests, they would have already rejected `.git`.
+	 */
 	if ((flags & GIT_PATH_REJECT_DOT_GIT_HFS) == 0 &&
 		(flags & GIT_PATH_REJECT_DOT_GIT_NTFS) == 0 &&
-		(flags & GIT_PATH_REJECT_DOT_GIT) &&
+		(flags & GIT_PATH_REJECT_DOT_GIT_LITERAL) &&
 		len == 4 &&
 		component[0] == '.' &&
 		(component[1] == 'g' || component[1] == 'G') &&
@@ -1649,6 +1652,8 @@ GIT_INLINE(unsigned int) dotgit_flags(
 {
 	int protectHFS = 0, protectNTFS = 0;
 
+	flags |= GIT_PATH_REJECT_DOT_GIT_LITERAL;
+
 #ifdef __APPLE__
 	protectHFS = 1;
 #endif

src/path.h

@@ -564,15 +564,16 @@ extern int git_path_from_url_or_path(git_buf *local_path_out, const char *url_or
 #define GIT_PATH_REJECT_TRAILING_COLON     (1 << 6)
 #define GIT_PATH_REJECT_DOS_PATHS          (1 << 7)
 #define GIT_PATH_REJECT_NT_CHARS           (1 << 8)
-#define GIT_PATH_REJECT_DOT_GIT_HFS        (1 << 9)
-#define GIT_PATH_REJECT_DOT_GIT_NTFS       (1 << 10)
+#define GIT_PATH_REJECT_DOT_GIT_LITERAL    (1 << 9)
+#define GIT_PATH_REJECT_DOT_GIT_HFS        (1 << 10)
+#define GIT_PATH_REJECT_DOT_GIT_NTFS       (1 << 11)
 
 /* Default path safety for writing files to disk: since we use the
  * Win32 "File Namespace" APIs ("\\?\") we need to protect from
  * paths that the normal Win32 APIs would not write.
  */
 #ifdef GIT_WIN32
-# define GIT_PATH_REJECT_DEFAULTS \
+# define GIT_PATH_REJECT_FILESYSTEM_DEFAULTS \
 	GIT_PATH_REJECT_TRAVERSAL | \
 	GIT_PATH_REJECT_BACKSLASH | \
 	GIT_PATH_REJECT_TRAILING_DOT | \
@@ -581,9 +582,18 @@ extern int git_path_from_url_or_path(git_buf *local_path_out, const char *url_or
 	GIT_PATH_REJECT_DOS_PATHS | \
 	GIT_PATH_REJECT_NT_CHARS
 #else
-# define GIT_PATH_REJECT_DEFAULTS GIT_PATH_REJECT_TRAVERSAL
+# define GIT_PATH_REJECT_FILESYSTEM_DEFAULTS \
+	GIT_PATH_REJECT_TRAVERSAL
 #endif
 
+ /* Paths that should never be written into the working directory. */
+#define GIT_PATH_REJECT_WORKDIR_DEFAULTS \
+	GIT_PATH_REJECT_FILESYSTEM_DEFAULTS | GIT_PATH_REJECT_DOT_GIT
+
+/* Paths that should never be written to the index. */
+#define GIT_PATH_REJECT_INDEX_DEFAULTS \
+	GIT_PATH_REJECT_TRAVERSAL | GIT_PATH_REJECT_DOT_GIT
+
 /*
  * Determine whether a path is a valid git path or not - this must not contain
  * a '.' or '..' component, or a component that is ".git" (in any case).

src/refdb_fs.c

@@ -717,7 +717,7 @@ static int loose_lock(git_filebuf *file, refdb_fs_backend *backend, const char *
 
 	assert(file && backend && name);
 
-	if (!git_path_isvalid(backend->repo, name, GIT_PATH_REJECT_DEFAULTS)) {
+	if (!git_path_isvalid(backend->repo, name, GIT_PATH_REJECT_FILESYSTEM_DEFAULTS)) {
 		giterr_set(GITERR_INVALID, "Invalid reference name '%s'.", name);
 		return GIT_EINVALIDSPEC;
 	}
@@ -1672,7 +1672,7 @@ static int lock_reflog(git_filebuf *file, refdb_fs_backend *backend, const char
 
 	repo = backend->repo;
 
-	if (!git_path_isvalid(backend->repo, refname, GIT_PATH_REJECT_DEFAULTS)) {
+	if (!git_path_isvalid(backend->repo, refname, GIT_PATH_REJECT_FILESYSTEM_DEFAULTS)) {
 		giterr_set(GITERR_INVALID, "Invalid reference name '%s'.", refname);
 		return GIT_EINVALIDSPEC;
 	}

tests/path/core.c

@@ -105,12 +105,12 @@ void test_path_core__isvalid_dot_git(void)
 	cl_assert_equal_b(true, git_path_isvalid(NULL, "foo/.GIT/bar", 0));
 	cl_assert_equal_b(true, git_path_isvalid(NULL, "foo/bar/.Git", 0));
 
-	cl_assert_equal_b(false, git_path_isvalid(NULL, ".git", GIT_PATH_REJECT_DOT_GIT));
-	cl_assert_equal_b(false, git_path_isvalid(NULL, ".git/foo", GIT_PATH_REJECT_DOT_GIT));
-	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.git", GIT_PATH_REJECT_DOT_GIT));
-	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.git/bar", GIT_PATH_REJECT_DOT_GIT));
-	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.GIT/bar", GIT_PATH_REJECT_DOT_GIT));
-	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/bar/.Git", GIT_PATH_REJECT_DOT_GIT));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, ".git", GIT_PATH_REJECT_DOT_GIT_LITERAL));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, ".git/foo", GIT_PATH_REJECT_DOT_GIT_LITERAL));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.git", GIT_PATH_REJECT_DOT_GIT_LITERAL));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.git/bar", GIT_PATH_REJECT_DOT_GIT_LITERAL));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/.GIT/bar", GIT_PATH_REJECT_DOT_GIT_LITERAL));
+	cl_assert_equal_b(false, git_path_isvalid(NULL, "foo/bar/.Git", GIT_PATH_REJECT_DOT_GIT_LITERAL));
 
 	cl_assert_equal_b(true, git_path_isvalid(NULL, "!git", 0));
 	cl_assert_equal_b(true, git_path_isvalid(NULL, "foo/!git", 0));

tests/win32/forbidden.c

@@ -0,0 +1,183 @@
+#include "clar_libgit2.h"
+
+#include "repository.h"
+#include "buffer.h"
+#include "submodule.h"
+
+static const char *repo_name = "win32-forbidden";
+static git_repository *repo;
+
+void test_win32_forbidden__initialize(void)
+{
+	repo = cl_git_sandbox_init(repo_name);
+}
+
+void test_win32_forbidden__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+void test_win32_forbidden__can_open_index(void)
+{
+	git_index *index;
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_assert_equal_i(7, git_index_entrycount(index));
+
+	/* ensure we can even write the unmodified index */
+	cl_git_pass(git_index_write(index));
+
+	git_index_free(index);
+}
+
+void test_win32_forbidden__can_add_forbidden_filename_with_entry(void)
+{
+	git_index *index;
+	git_index_entry entry = {{0}};
+
+	cl_git_pass(git_repository_index(&index, repo));
+
+	entry.path = "aux";
+	entry.mode = GIT_FILEMODE_BLOB;
+	git_oid_fromstr(&entry.id, "da623abd956bb2fd8052c708c7ed43f05d192d37");
+
+	cl_git_pass(git_index_add(index, &entry));
+
+	git_index_free(index);
+}
+
+void test_win32_forbidden__cannot_add_dot_git_even_with_entry(void)
+{
+	git_index *index;
+	git_index_entry entry = {{0}};
+
+	cl_git_pass(git_repository_index(&index, repo));
+
+	entry.path = "foo/.git";
+	entry.mode = GIT_FILEMODE_BLOB;
+	git_oid_fromstr(&entry.id, "da623abd956bb2fd8052c708c7ed43f05d192d37");
+
+	cl_git_fail(git_index_add(index, &entry));
+
+	git_index_free(index);
+}
+
+void test_win32_forbidden__cannot_add_forbidden_filename_from_filesystem(void)
+{
+	git_index *index;
+
+	/* since our function calls are very low-level, we can create `aux.`,
+	 * but we should not be able to add it to the index
+	 */
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_write2file("win32-forbidden/aux.", "foo\n", 4, O_RDWR | O_CREAT, 0666);
+
+#ifdef GIT_WIN32
+	cl_git_fail(git_index_add_bypath(index, "aux."));
+#else
+	cl_git_pass(git_index_add_bypath(index, "aux."));
+#endif
+
+	cl_must_pass(p_unlink("win32-forbidden/aux."));
+	git_index_free(index);
+}
+
+static int dummy_submodule_cb(
+	git_submodule *sm, const char *name, void *payload)
+{
+	GIT_UNUSED(sm);
+	GIT_UNUSED(name);
+	GIT_UNUSED(payload);
+	return 0;
+}
+
+void test_win32_forbidden__can_diff_tree_to_index(void)
+{
+	git_diff *diff;
+	git_tree *tree;
+
+	cl_git_pass(git_repository_head_tree(&tree, repo));
+	cl_git_pass(git_diff_tree_to_index(&diff, repo, tree, NULL, NULL));
+	cl_assert_equal_i(0, git_diff_num_deltas(diff));
+	git_diff_free(diff);
+	git_tree_free(tree);
+}
+
+void test_win32_forbidden__can_diff_tree_to_tree(void)
+{
+	git_diff *diff;
+	git_tree *tree;
+
+	cl_git_pass(git_repository_head_tree(&tree, repo));
+	cl_git_pass(git_diff_tree_to_tree(&diff, repo, tree, tree, NULL));
+	cl_assert_equal_i(0, git_diff_num_deltas(diff));
+	git_diff_free(diff);
+	git_tree_free(tree);
+}
+
+void test_win32_forbidden__can_diff_index_to_workdir(void)
+{
+	git_index *index;
+	git_diff *diff;
+	const git_diff_delta *delta;
+	git_tree *tree;
+	size_t i;
+
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_pass(git_repository_head_tree(&tree, repo));
+	cl_git_pass(git_diff_index_to_workdir(&diff, repo, index, NULL));
+
+	for (i = 0; i < git_diff_num_deltas(diff); i++) {
+		delta = git_diff_get_delta(diff, i);
+		cl_assert_equal_i(GIT_DELTA_DELETED, delta->status);
+	}
+
+	git_diff_free(diff);
+	git_tree_free(tree);
+	git_index_free(index);
+}
+
+void test_win32_forbidden__checking_out_forbidden_index_fails(void)
+{
+#ifdef GIT_WIN32
+	git_index *index;
+	git_checkout_options opts = GIT_CHECKOUT_OPTIONS_INIT;
+	git_diff *diff;
+	const git_diff_delta *delta;
+	git_tree *tree;
+	size_t num_deltas, i;
+
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_fail(git_checkout_index(repo, index, &opts));
+
+	cl_git_pass(git_repository_head_tree(&tree, repo));
+	cl_git_pass(git_diff_index_to_workdir(&diff, repo, index, NULL));
+
+	num_deltas = git_diff_num_deltas(diff);
+
+	cl_assert(num_deltas > 0);
+
+	for (i = 0; i < num_deltas; i++) {
+		delta = git_diff_get_delta(diff, i);
+		cl_assert_equal_i(GIT_DELTA_DELETED, delta->status);
+	}
+
+	git_diff_free(diff);
+	git_tree_free(tree);
+	git_index_free(index);
+#endif
+}
+
+void test_win32_forbidden__can_query_submodules(void)
+{
+	cl_git_pass(git_submodule_foreach(repo, dummy_submodule_cb, NULL));
+}
+
+void test_win32_forbidden__can_blame_file(void)
+{
+	git_blame *blame;
+
+	cl_git_pass(git_blame_file(&blame, repo, "aux", NULL));
+	git_blame_free(blame);
+}