Commit 6ddf533afca5e25b7c532b4c164ae66e06f9c1c2

Edward Thomson 2016-02-23T18:29:16

git_index_add: validate objects in index entries (optionally) When `GIT_OPT_ENABLE_STRICT_OBJECT_CREATION` is turned on, validate the index entries given to `git_index_add`.

diff --git a/src/index.c b/src/index.c
index 5704432..b97f809 100644
--- a/src/index.c
+++ b/src/index.c
@@ -1245,17 +1245,22 @@ static void index_existing_and_best(
  * it, then it will return an error **and also free the entry**.  When
  * it replaces an existing entry, it will update the entry_ptr with the
  * actual entry in the index (and free the passed in one).
+ *
  * trust_path is whether we use the given path, or whether (on case
  * insensitive systems only) we try to canonicalize the given path to
  * be within an existing directory.
+ *
  * trust_mode is whether we trust the mode in entry_ptr.
+ *
+ * trust_id is whether we trust the id or it should be validated.
  */
 static int index_insert(
 	git_index *index,
 	git_index_entry **entry_ptr,
 	int replace,
 	bool trust_path,
-	bool trust_mode)
+	bool trust_mode,
+	bool trust_id)
 {
 	int error = 0;
 	size_t path_length, position;
@@ -1288,6 +1293,15 @@ static int index_insert(
 	if (!trust_path)
 		error = canonicalize_directory_path(index, entry, best);
 
+	/* ensure that the given id exists (unless it's a submodule) */
+	if (!error && !trust_id && INDEX_OWNER(index) &&
+		(entry->mode & GIT_FILEMODE_COMMIT) != GIT_FILEMODE_COMMIT) {
+
+		if (!git_object__is_valid(INDEX_OWNER(index), &entry->id,
+			git_object__type_from_filemode(entry->mode)))
+			error = -1;
+	}
+
 	/* look for tree / blob name collisions, removing conflicts if requested */
 	if (!error)
 		error = check_file_directory_collision(index, entry, position, replace);
@@ -1395,7 +1409,7 @@ int git_index_add_frombuffer(
 	git_oid_cpy(&entry->id, &id);
 	entry->file_size = len;
 
-	if ((error = index_insert(index, &entry, 1, true, true)) < 0)
+	if ((error = index_insert(index, &entry, 1, true, true, true)) < 0)
 		return error;
 
 	/* Adding implies conflict was resolved, move conflict entries to REUC */
@@ -1454,7 +1468,7 @@ int git_index_add_bypath(git_index *index, const char *path)
 	assert(index && path);
 
 	if ((ret = index_entry_init(&entry, index, path)) == 0)
-		ret = index_insert(index, &entry, 1, false, false);
+		ret = index_insert(index, &entry, 1, false, false, true);
 
 	/* If we were given a directory, let's see if it's a submodule */
 	if (ret < 0 && ret != GIT_EDIRECTORY)
@@ -1480,7 +1494,7 @@ int git_index_add_bypath(git_index *index, const char *path)
 			if ((ret = add_repo_as_submodule(&entry, index, path)) < 0)
 				return ret;
 
-			if ((ret = index_insert(index, &entry, 1, false, false)) < 0)
+			if ((ret = index_insert(index, &entry, 1, false, false, true)) < 0)
 				return ret;
 		} else if (ret < 0) {
 			return ret;
@@ -1569,7 +1583,7 @@ int git_index_add(git_index *index, const git_index_entry *source_entry)
 	}
 
 	if ((ret = index_entry_dup(&entry, index, source_entry)) < 0 ||
-		(ret = index_insert(index, &entry, 1, true, true)) < 0)
+		(ret = index_insert(index, &entry, 1, true, true, false)) < 0)
 		return ret;
 
 	git_tree_cache_invalidate_path(index->tree, entry->path);
@@ -1731,7 +1745,7 @@ int git_index_conflict_add(git_index *index,
 		/* Make sure stage is correct */
 		GIT_IDXENTRY_STAGE_SET(entries[i], i + 1);
 
-		if ((ret = index_insert(index, &entries[i], 1, true, true)) < 0)
+		if ((ret = index_insert(index, &entries[i], 1, true, true, false)) < 0)
 			goto on_error;
 
 		entries[i] = NULL; /* don't free if later entry fails */
diff --git a/src/object.h b/src/object.h
index 7824360..13edf31 100644
--- a/src/object.h
+++ b/src/object.h
@@ -46,4 +46,20 @@ GIT_INLINE(bool) git_object__is_valid(
 	return valid;
 }
 
+GIT_INLINE(git_otype) git_object__type_from_filemode(git_filemode_t mode)
+{
+	switch (mode) {
+	case GIT_FILEMODE_TREE:
+		return GIT_OBJ_TREE;
+	case GIT_FILEMODE_COMMIT:
+		return GIT_OBJ_COMMIT;
+	case GIT_FILEMODE_BLOB:
+	case GIT_FILEMODE_BLOB_EXECUTABLE:
+	case GIT_FILEMODE_LINK:
+		return GIT_OBJ_BLOB;
+	default:
+		return GIT_OBJ_BAD;
+	}
+}
+
 #endif
diff --git a/tests/index/add.c b/tests/index/add.c
new file mode 100644
index 0000000..cfa81c4
--- /dev/null
+++ b/tests/index/add.c
@@ -0,0 +1,84 @@
+#include "clar_libgit2.h"
+
+static git_repository *g_repo = NULL;
+static git_index *g_index = NULL;
+
+static const char *valid_blob_id = "fa49b077972391ad58037050f2a75f74e3671e92";
+static const char *valid_tree_id = "181037049a54a1eb5fab404658a3a250b44335d7";
+static const char *valid_commit_id = "763d71aadf09a7951596c9746c024e7eece7c7af";
+static const char *invalid_id = "1234567890123456789012345678901234567890";
+
+void test_index_add__initialize(void)
+{
+	g_repo = cl_git_sandbox_init("testrepo");
+	cl_git_pass(git_repository_index(&g_index, g_repo));
+}
+
+void test_index_add__cleanup(void)
+{
+	git_index_free(g_index);
+	cl_git_sandbox_cleanup();
+	g_repo = NULL;
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0));
+}
+
+static void test_add_entry(
+	bool should_succeed, const char *idstr, git_filemode_t mode)
+{
+	git_index_entry entry = {{0}};
+
+	cl_git_pass(git_oid_fromstr(&entry.id, idstr));
+
+	entry.path = mode == GIT_FILEMODE_TREE ? "test_folder" : "test_file";
+	entry.mode = mode;
+
+	if (should_succeed)
+		cl_git_pass(git_index_add(g_index, &entry));
+	else
+		cl_git_fail(git_index_add(g_index, &entry));
+}
+
+void test_index_add__invalid_entries_succeeds_by_default(void)
+{
+	/*
+	 * Ensure that there is no validation on ids by default
+	 */
+
+	/* ensure that we can add some actually good entries */
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_BLOB);
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_LINK);
+
+	/* test that we fail to add some invalid (missing) blobs and trees */
+	test_add_entry(true, invalid_id, GIT_FILEMODE_BLOB);
+	test_add_entry(true, invalid_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(true, invalid_id, GIT_FILEMODE_LINK);
+
+	/* test that we validate the types of objects */
+	test_add_entry(true, valid_commit_id, GIT_FILEMODE_BLOB);
+	test_add_entry(true, valid_tree_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(true, valid_commit_id, GIT_FILEMODE_LINK);
+
+	/*
+	 * Ensure that strict object references will fail the `index_add`
+	 */
+
+	cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1));
+
+	/* ensure that we can add some actually good entries */
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_BLOB);
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(true, valid_blob_id, GIT_FILEMODE_LINK);
+
+	/* test that we fail to add some invalid (missing) blobs and trees */
+	test_add_entry(false, invalid_id, GIT_FILEMODE_BLOB);
+	test_add_entry(false, invalid_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(false, invalid_id, GIT_FILEMODE_LINK);
+
+	/* test that we validate the types of objects */
+	test_add_entry(false, valid_commit_id, GIT_FILEMODE_BLOB);
+	test_add_entry(false, valid_tree_id, GIT_FILEMODE_BLOB_EXECUTABLE);
+	test_add_entry(false, valid_commit_id, GIT_FILEMODE_LINK);
+}
+