From 22a19f5b5795153b4c77c75adfae790c3b919be4 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 22 Feb 2016 23:46:50 -0500 Subject: [PATCH 1/9] git_libgit2_opts: introduce `GIT_OPT_ENABLE_STRICT_OBJECT_CREATION` --- include/git2/common.h | 9 +++++++++ src/object.c | 2 ++ src/object.h | 2 ++ src/settings.c | 6 ++++++ 4 files changed, 19 insertions(+) diff --git a/include/git2/common.h b/include/git2/common.h index c26030840..4f43185f8 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -147,6 +147,7 @@ typedef enum { GIT_OPT_SET_TEMPLATE_PATH, GIT_OPT_SET_SSL_CERT_LOCATIONS, GIT_OPT_SET_USER_AGENT, + GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, } git_libgit2_opt_t; /** @@ -251,6 +252,14 @@ typedef enum { * > - `user_agent` is the value that will be delivered as the * > User-Agent header on HTTP requests. * + * * opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, int enabled) + * + * > Enable strict input validation when creating new objects + * > to ensure that all inputs to the new objects are valid. For + * > example, when this is enabled, the parent(s) and tree inputs + * > will be validated when creating a new commit. This defaults + * > to disabled. + * * @param option Option key * @param ... value to set the option * @return 0 on success, <0 on failure diff --git a/src/object.c b/src/object.c index b0a8199bc..7f7de9fee 100644 --- a/src/object.c +++ b/src/object.c @@ -14,6 +14,8 @@ #include "blob.h" #include "tag.h" +bool git_object__strict_input_validation = false; + typedef struct { const char *str; /* type name string */ size_t size; /* size in bytes of the object structure */ diff --git a/src/object.h b/src/object.h index d187c55b7..358279a3d 100644 --- a/src/object.h +++ b/src/object.h @@ -7,6 +7,8 @@ #ifndef INCLUDE_object_h__ #define INCLUDE_object_h__ +extern bool git_object__strict_input_validation; + /** Base git object for inheritance */ struct git_object { git_cached_obj cached; diff --git a/src/settings.c b/src/settings.c index d7341abe8..88602bad0 100644 --- a/src/settings.c +++ b/src/settings.c @@ -14,6 +14,7 @@ #include "sysdir.h" #include "cache.h" #include "global.h" +#include "object.h" void git_libgit2_version(int *major, int *minor, int *rev) { @@ -181,6 +182,11 @@ int git_libgit2_opts(int key, ...) } break; + + case GIT_OPT_ENABLE_STRICT_OBJECT_CREATION: + git_object__strict_input_validation = (va_arg(ap, int) != 0); + break; + default: giterr_set(GITERR_INVALID, "invalid option key"); error = -1; From 7565dc6572e94ee10462169dd07ba8afe08388c7 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 23 Feb 2016 13:33:10 -0500 Subject: [PATCH 2/9] git_object__is_valid: simple object validity test --- src/object.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/object.h b/src/object.h index 358279a3d..782436017 100644 --- a/src/object.h +++ b/src/object.h @@ -30,4 +30,20 @@ int git_oid__parse(git_oid *oid, const char **buffer_out, const char *buffer_end void git_oid__writebuf(git_buf *buf, const char *header, const git_oid *oid); +GIT_INLINE(bool) git_object__is_valid( + git_repository *repo, const git_oid *id, git_otype type) +{ + git_object *obj = NULL; + bool valid = true; + + if (git_object__strict_input_validation) { + if (git_object_lookup(&obj, repo, id, type) < 0) + valid = false; + + git_object_free(obj); + } + + return valid; +} + #endif From ef63bab306a2a85d15e62bfb73f49ae11f2b5df6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 23 Feb 2016 13:34:35 -0500 Subject: [PATCH 3/9] git_commit: validate tree and parent ids When `GIT_OPT_ENABLE_STRICT_OBJECT_CREATION` is turned on, validate the tree and parent ids given to commit creation functions. --- src/commit.c | 48 ++++++++++++++----- tests/commit/write.c | 111 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 143 insertions(+), 16 deletions(-) diff --git a/src/commit.c b/src/commit.c index 5a0509803..685c642aa 100644 --- a/src/commit.c +++ b/src/commit.c @@ -17,6 +17,7 @@ #include "signature.h" #include "message.h" #include "refs.h" +#include "object.h" void git_commit__free(void *_commit) { @@ -36,7 +37,7 @@ void git_commit__free(void *_commit) git__free(commit); } -int git_commit_create_from_callback( +static int git_commit__create_internal( git_oid *id, git_repository *repo, const char *update_ref, @@ -46,7 +47,8 @@ int git_commit_create_from_callback( const char *message, const git_oid *tree, git_commit_parent_callback parent_cb, - void *parent_payload) + void *parent_payload, + bool validate) { git_reference *ref = NULL; int error = 0, matched_parent = 0; @@ -58,6 +60,9 @@ int git_commit_create_from_callback( assert(id && repo && tree && parent_cb); + if (validate && !git_object__is_valid(repo, tree, GIT_OBJ_TREE)) + return -1; + if (update_ref) { error = git_reference_lookup_resolved(&ref, repo, update_ref, 10); if (error < 0 && error != GIT_ENOTFOUND) @@ -71,6 +76,11 @@ int git_commit_create_from_callback( git_oid__writebuf(&commit, "tree ", tree); while ((parent = parent_cb(i, parent_payload)) != NULL) { + if (validate && !git_object__is_valid(repo, parent, GIT_OBJ_COMMIT)) { + error = -1; + goto on_error; + } + git_oid__writebuf(&commit, "parent ", parent); if (i == 0 && current_id && git_oid_equal(current_id, parent)) matched_parent = 1; @@ -114,10 +124,26 @@ int git_commit_create_from_callback( on_error: git_buf_free(&commit); - giterr_set(GITERR_OBJECT, "Failed to create commit."); return -1; } +int git_commit_create_from_callback( + git_oid *id, + git_repository *repo, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_oid *tree, + git_commit_parent_callback parent_cb, + void *parent_payload) +{ + return git_commit__create_internal( + id, repo, update_ref, author, committer, message_encoding, message, + tree, parent_cb, parent_payload, true); +} + typedef struct { size_t total; va_list args; @@ -153,10 +179,10 @@ int git_commit_create_v( data.total = parent_count; va_start(data.args, parent_count); - error = git_commit_create_from_callback( + error = git_commit__create_internal( id, repo, update_ref, author, committer, message_encoding, message, git_tree_id(tree), - commit_parent_from_varargs, &data); + commit_parent_from_varargs, &data, false); va_end(data.args); return error; @@ -187,10 +213,10 @@ int git_commit_create_from_ids( { commit_parent_oids data = { parent_count, parents }; - return git_commit_create_from_callback( + return git_commit__create_internal( id, repo, update_ref, author, committer, message_encoding, message, tree, - commit_parent_from_ids, &data); + commit_parent_from_ids, &data, true); } typedef struct { @@ -227,10 +253,10 @@ int git_commit_create( assert(tree && git_tree_owner(tree) == repo); - return git_commit_create_from_callback( + return git_commit__create_internal( id, repo, update_ref, author, committer, message_encoding, message, git_tree_id(tree), - commit_parent_from_array, &data); + commit_parent_from_array, &data, false); } static const git_oid *commit_parent_for_amend(size_t curr, void *payload) @@ -290,9 +316,9 @@ int git_commit_amend( } } - error = git_commit_create_from_callback( + error = git_commit__create_internal( id, repo, NULL, author, committer, message_encoding, message, - &tree_id, commit_parent_for_amend, (void *)commit_to_amend); + &tree_id, commit_parent_for_amend, (void *)commit_to_amend, false); if (!error && update_ref) { error = git_reference__update_for_commit( diff --git a/tests/commit/write.c b/tests/commit/write.c index 176965cbd..303d1ce58 100644 --- a/tests/commit/write.c +++ b/tests/commit/write.c @@ -1,10 +1,12 @@ #include "clar_libgit2.h" +#include "git2/sys/commit.h" static const char *committer_name = "Vicent Marti"; static const char *committer_email = "vicent@github.com"; static const char *commit_message = "This commit has been created in memory\n\ This is a commit created in memory and it will be written back to disk\n"; -static const char *tree_oid = "1810dff58d8a660512d4832e740f692884338ccd"; +static const char *tree_id_str = "1810dff58d8a660512d4832e740f692884338ccd"; +static const char *parent_id_str = "8496071c1b46c854b31185ea97743be6a8774479"; static const char *root_commit_message = "This is a root commit\n\ This is a root commit and should be the only one in this branch\n"; static const char *root_reflog_message = "commit (initial): This is a root commit \ @@ -35,6 +37,8 @@ void test_commit_write__cleanup(void) head_old = NULL; cl_git_sandbox_cleanup(); + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); } @@ -46,12 +50,11 @@ void test_commit_write__from_memory(void) const git_signature *author1, *committer1; git_commit *parent; git_tree *tree; - const char *commit_id_str = "8496071c1b46c854b31185ea97743be6a8774479"; - git_oid_fromstr(&tree_id, tree_oid); + git_oid_fromstr(&tree_id, tree_id_str); cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id)); - git_oid_fromstr(&parent_id, commit_id_str); + git_oid_fromstr(&parent_id, parent_id_str); cl_git_pass(git_commit_lookup(&parent, g_repo, &parent_id)); /* create signatures */ @@ -106,7 +109,7 @@ void test_commit_write__root(void) git_reflog *log; const git_reflog_entry *entry; - git_oid_fromstr(&tree_id, tree_oid); + git_oid_fromstr(&tree_id, tree_id_str); cl_git_pass(git_tree_lookup(&tree, g_repo, &tree_id)); /* create signatures */ @@ -158,3 +161,101 @@ void test_commit_write__root(void) git_signature_free(committer); git_reflog_free(log); } + +static int create_commit_from_ids( + git_oid *result, + const git_oid *tree_id, + const git_oid *parent_id) +{ + git_signature *author, *committer; + const git_oid *parent_ids[1]; + int ret; + + cl_git_pass(git_signature_new( + &committer, committer_name, committer_email, 123456789, 60)); + cl_git_pass(git_signature_new( + &author, committer_name, committer_email, 987654321, 90)); + + parent_ids[0] = parent_id; + + ret = git_commit_create_from_ids( + result, + g_repo, + NULL, + author, + committer, + NULL, + root_commit_message, + tree_id, + 1, + parent_ids); + + git_signature_free(committer); + git_signature_free(author); + + return ret; +} + +void test_commit_write__doesnt_validate_objects_by_default(void) +{ + git_oid expected_id, tree_id, parent_id, commit_id; + + /* this is a valid tree and parent */ + git_oid_fromstr(&tree_id, tree_id_str); + git_oid_fromstr(&parent_id, parent_id_str); + + git_oid_fromstr(&expected_id, "c8571bbec3a72c4bcad31648902e5a453f1adece"); + cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + cl_assert_equal_oid(&expected_id, &commit_id); + + /* this is a wholly invented tree id */ + git_oid_fromstr(&tree_id, "1234567890123456789012345678901234567890"); + git_oid_fromstr(&parent_id, parent_id_str); + + git_oid_fromstr(&expected_id, "996008340b8e68d69bf3c28d7c57fb7ec3c8e202"); + cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + cl_assert_equal_oid(&expected_id, &commit_id); + + /* this is a wholly invented parent id */ + git_oid_fromstr(&tree_id, tree_id_str); + git_oid_fromstr(&parent_id, "1234567890123456789012345678901234567890"); + + git_oid_fromstr(&expected_id, "d78f660cab89d9791ca6714b57978bf2a7e709fd"); + cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + cl_assert_equal_oid(&expected_id, &commit_id); + + /* these are legitimate objects, but of the wrong type */ + git_oid_fromstr(&tree_id, parent_id_str); + git_oid_fromstr(&parent_id, tree_id_str); + + git_oid_fromstr(&expected_id, "5d80c07414e3f18792949699dfcacadf7748f361"); + cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + cl_assert_equal_oid(&expected_id, &commit_id); +} + +void test_commit_write__can_validate_objects(void) +{ + git_oid tree_id, parent_id, commit_id; + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); + + /* this is a valid tree and parent */ + git_oid_fromstr(&tree_id, tree_id_str); + git_oid_fromstr(&parent_id, parent_id_str); + cl_git_pass(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + + /* this is a wholly invented tree id */ + git_oid_fromstr(&tree_id, "1234567890123456789012345678901234567890"); + git_oid_fromstr(&parent_id, parent_id_str); + cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + + /* this is a wholly invented parent id */ + git_oid_fromstr(&tree_id, tree_id_str); + git_oid_fromstr(&parent_id, "1234567890123456789012345678901234567890"); + cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); + + /* these are legitimate objects, but of the wrong type */ + git_oid_fromstr(&tree_id, parent_id_str); + git_oid_fromstr(&parent_id, tree_id_str); + cl_git_fail(create_commit_from_ids(&commit_id, &tree_id, &parent_id)); +} From 2bbc7d3e564ed262e9555ea4fd081ece5ceb3bff Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 23 Feb 2016 15:00:27 -0500 Subject: [PATCH 4/9] treebuilder: validate tree entries (optionally) When `GIT_OPT_ENABLE_STRICT_OBJECT_CREATION` is turned on, validate the tree and parent ids given to treebuilder insertion. --- src/tree.c | 15 +++++++++++ tests/object/tree/write.c | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/tree.c b/src/tree.c index cfceb3f33..2c3151546 100644 --- a/src/tree.c +++ b/src/tree.c @@ -726,6 +726,18 @@ on_error: return -1; } +static git_otype otype_from_mode(git_filemode_t filemode) +{ + switch (filemode) { + case GIT_FILEMODE_TREE: + return GIT_OBJ_TREE; + case GIT_FILEMODE_COMMIT: + return GIT_OBJ_COMMIT; + default: + return GIT_OBJ_BLOB; + } +} + int git_treebuilder_insert( const git_tree_entry **entry_out, git_treebuilder *bld, @@ -745,6 +757,9 @@ int git_treebuilder_insert( if (!valid_entry_name(bld->repo, filename)) return tree_error("Failed to insert entry. Invalid name for a tree entry", filename); + if (!git_object__is_valid(bld->repo, id, otype_from_mode(filemode))) + return tree_error("Failed to insert entry; invalid object specified", filename); + pos = git_strmap_lookup_index(bld->map, filename); if (git_strmap_valid_index(bld->map, pos)) { entry = git_strmap_value_at(bld->map, pos); diff --git a/tests/object/tree/write.c b/tests/object/tree/write.c index 5433e5f03..4cd1607d8 100644 --- a/tests/object/tree/write.c +++ b/tests/object/tree/write.c @@ -18,6 +18,8 @@ void test_object_tree_write__initialize(void) void test_object_tree_write__cleanup(void) { cl_git_sandbox_cleanup(); + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); } void test_object_tree_write__from_memory(void) @@ -440,3 +442,56 @@ void test_object_tree_write__protect_filesystems(void) git_treebuilder_free(builder); } + +static void test_invalid_objects(bool should_allow_invalid) +{ + git_treebuilder *builder; + git_oid valid_blob_id, invalid_blob_id, valid_tree_id, invalid_tree_id; + +#define assert_allowed(expr) \ + clar__assert(!(expr) == should_allow_invalid, __FILE__, __LINE__, \ + (should_allow_invalid ? \ + "Expected function call to succeed: " #expr : \ + "Expected function call to fail: " #expr), \ + NULL, 1) + + cl_git_pass(git_oid_fromstr(&valid_blob_id, blob_oid)); + cl_git_pass(git_oid_fromstr(&invalid_blob_id, + "1234567890123456789012345678901234567890")); + cl_git_pass(git_oid_fromstr(&valid_tree_id, first_tree)); + cl_git_pass(git_oid_fromstr(&invalid_tree_id, + "0000000000111111111122222222223333333333")); + + cl_git_pass(git_treebuilder_new(&builder, g_repo, NULL)); + + /* test valid blobs and trees (these should always pass) */ + cl_git_pass(git_treebuilder_insert(NULL, builder, "file.txt", &valid_blob_id, GIT_FILEMODE_BLOB)); + cl_git_pass(git_treebuilder_insert(NULL, builder, "folder", &valid_tree_id, GIT_FILEMODE_TREE)); + + /* replace valid files and folders with invalid ones */ + assert_allowed(git_treebuilder_insert(NULL, builder, "file.txt", &invalid_blob_id, GIT_FILEMODE_BLOB)); + assert_allowed(git_treebuilder_insert(NULL, builder, "folder", &invalid_blob_id, GIT_FILEMODE_BLOB)); + + /* insert new invalid files and folders */ + assert_allowed(git_treebuilder_insert(NULL, builder, "invalid_file.txt", &invalid_blob_id, GIT_FILEMODE_BLOB)); + assert_allowed(git_treebuilder_insert(NULL, builder, "invalid_folder", &invalid_blob_id, GIT_FILEMODE_BLOB)); + + /* insert valid blobs as trees and trees as blobs */ + assert_allowed(git_treebuilder_insert(NULL, builder, "file_as_folder", &valid_blob_id, GIT_FILEMODE_TREE)); + assert_allowed(git_treebuilder_insert(NULL, builder, "folder_as_file.txt", &valid_tree_id, GIT_FILEMODE_BLOB)); + +#undef assert_allowed + + git_treebuilder_free(builder); +} + +void test_object_tree_write__object_validity(void) +{ + /* Ensure that we can add invalid objects by default */ + test_invalid_objects(true); + + /* Ensure that we can turn on validation */ + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); + test_invalid_objects(false); +} + From 6ddf533afca5e25b7c532b4c164ae66e06f9c1c2 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 23 Feb 2016 18:29:16 -0500 Subject: [PATCH 5/9] 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`. --- src/index.c | 26 +++++++++++---- src/object.h | 16 +++++++++ tests/index/add.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 tests/index/add.c diff --git a/src/index.c b/src/index.c index 5704432ae..b97f8091d 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 782436017..13edf3118 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 000000000..cfa81c4d9 --- /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); +} + From 3ef01e772729f44c2871b590577cc64e4a58a381 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sun, 28 Feb 2016 14:37:37 -0500 Subject: [PATCH 6/9] git_object__is_valid: use `odb_read_header` This allows lighter weight validation in `git_object__is_valid` that does not require reading the entire object. --- src/object.c | 24 ++++++++++++++++++++++++ src/object.h | 19 ++++--------------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/object.c b/src/object.c index 7f7de9fee..e7c1fef09 100644 --- a/src/object.c +++ b/src/object.c @@ -467,3 +467,27 @@ int git_object_short_id(git_buf *out, const git_object *obj) return error; } +bool git_object__is_valid( + git_repository *repo, const git_oid *id, git_otype expected_type) +{ + git_odb *odb; + git_otype actual_type; + size_t len; + int error; + + if (!git_object__strict_input_validation) + return true; + + if ((error = git_repository_odb__weakptr(&odb, repo)) < 0 || + (error = git_odb_read_header(&len, &actual_type, odb, id)) < 0) + return false; + + if (expected_type != GIT_OBJ_ANY && expected_type != actual_type) { + giterr_set(GITERR_INVALID, + "the requested type does not match the type in the ODB"); + return false; + } + + return true; +} + diff --git a/src/object.h b/src/object.h index 13edf3118..dd227d16d 100644 --- a/src/object.h +++ b/src/object.h @@ -7,6 +7,8 @@ #ifndef INCLUDE_object_h__ #define INCLUDE_object_h__ +#include "repository.h" + extern bool git_object__strict_input_validation; /** Base git object for inheritance */ @@ -30,21 +32,8 @@ int git_oid__parse(git_oid *oid, const char **buffer_out, const char *buffer_end void git_oid__writebuf(git_buf *buf, const char *header, const git_oid *oid); -GIT_INLINE(bool) git_object__is_valid( - git_repository *repo, const git_oid *id, git_otype type) -{ - git_object *obj = NULL; - bool valid = true; - - if (git_object__strict_input_validation) { - if (git_object_lookup(&obj, repo, id, type) < 0) - valid = false; - - git_object_free(obj); - } - - return valid; -} +bool git_object__is_valid( + git_repository *repo, const git_oid *id, git_otype expected_type); GIT_INLINE(git_otype) git_object__type_from_filemode(git_filemode_t mode) { From 98c341496f276aa93737fd6ac71a23b77249d82d Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sun, 28 Feb 2016 15:11:15 -0500 Subject: [PATCH 7/9] refs: honor strict object creation --- src/refs.c | 8 +------- tests/refs/create.c | 26 ++++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/refs.c b/src/refs.c index 7b538659d..a15e31b53 100644 --- a/src/refs.c +++ b/src/refs.c @@ -377,15 +377,9 @@ static int reference__create( return error; if (oid != NULL) { - git_odb *odb; - assert(symbolic == NULL); - /* Sanity check the reference being created - target must exist. */ - if ((error = git_repository_odb__weakptr(&odb, repo)) < 0) - return error; - - if (!git_odb_exists(odb, oid)) { + if (!git_object__is_valid(repo, oid, GIT_OBJ_ANY)) { giterr_set(GITERR_REFERENCE, "Target OID for the reference doesn't exist on the repository"); return -1; diff --git a/tests/refs/create.c b/tests/refs/create.c index 48194ae3b..a0bc78014 100644 --- a/tests/refs/create.c +++ b/tests/refs/create.c @@ -18,6 +18,8 @@ void test_refs_create__initialize(void) void test_refs_create__cleanup(void) { cl_git_sandbox_cleanup(); + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); } void test_refs_create__symbolic(void) @@ -119,9 +121,9 @@ void test_refs_create__oid(void) git_reference_free(looked_up_ref); } -void test_refs_create__oid_unknown(void) +/* Can by default create a reference that targets at an unknown id */ +void test_refs_create__oid_unknown_succeeds_by_default(void) { - // Can not create a new OID reference which targets at an unknown id git_reference *new_reference, *looked_up_ref; git_oid id; @@ -129,6 +131,26 @@ void test_refs_create__oid_unknown(void) git_oid_fromstr(&id, "deadbeef3f795b2b4353bcce3a527ad0a4f7f644"); + /* Create and write the new object id reference */ + cl_git_pass(git_reference_create(&new_reference, g_repo, new_head, &id, 0, NULL)); + + /* Ensure the reference can't be looked-up... */ + cl_git_pass(git_reference_lookup(&looked_up_ref, g_repo, new_head)); + git_reference_free(looked_up_ref); +} + +/* Strict object enforcement enforces valid object id */ +void test_refs_create__oid_unknown_fails_strict_mode(void) +{ + git_reference *new_reference, *looked_up_ref; + git_oid id; + + const char *new_head = "refs/heads/new-head"; + + git_oid_fromstr(&id, "deadbeef3f795b2b4353bcce3a527ad0a4f7f644"); + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); + /* Create and write the new object id reference */ cl_git_fail(git_reference_create(&new_reference, g_repo, new_head, &id, 0, NULL)); From 4afe536ba1c909ff8ab8d1b75997d3897b72571b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Sun, 28 Feb 2016 16:02:49 -0500 Subject: [PATCH 8/9] tests: use legitimate object ids Use legitimate (existing) object IDs in tests so that we have the ability to turn on strict object validation when running tests. --- tests/diff/index.c | 8 ++--- tests/diff/workdir.c | 8 +++-- tests/index/bypath.c | 3 ++ tests/index/cache.c | 6 ++-- tests/index/conflicts.c | 34 +++++++----------- tests/index/filemodes.c | 7 ++++ tests/index/racy.c | 4 +++ tests/object/tree/write.c | 31 +++++++++------- .../2b/d0a343aeef7a2cf0d158478966a6e587ff3863 | Bin 0 -> 56 bytes .../d4/27e0b2e138501a3d15cc376077a3631e15bd46 | Bin 0 -> 38 bytes .../ee/3fa1b8c00aff7fe02065fdb50864bb0d932ccf | Bin 0 -> 64 bytes .../ee/3fa1b8c00aff7fe02065fdb50864bb0d932ccf | Bin 0 -> 64 bytes 12 files changed, 57 insertions(+), 44 deletions(-) create mode 100644 tests/resources/status/.gitted/objects/2b/d0a343aeef7a2cf0d158478966a6e587ff3863 create mode 100644 tests/resources/status/.gitted/objects/d4/27e0b2e138501a3d15cc376077a3631e15bd46 create mode 100644 tests/resources/status/.gitted/objects/ee/3fa1b8c00aff7fe02065fdb50864bb0d932ccf create mode 100644 tests/resources/testrepo/.gitted/objects/ee/3fa1b8c00aff7fe02065fdb50864bb0d932ccf diff --git a/tests/diff/index.c b/tests/diff/index.c index df45ad236..0293b7821 100644 --- a/tests/diff/index.c +++ b/tests/diff/index.c @@ -185,9 +185,9 @@ static void do_conflicted_diff(diff_expects *exp, unsigned long flags) ancestor.path = ours.path = theirs.path = "staged_changes"; ancestor.mode = ours.mode = theirs.mode = GIT_FILEMODE_BLOB; - git_oid_fromstr(&ancestor.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); - git_oid_fromstr(&ours.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); - git_oid_fromstr(&theirs.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + git_oid_fromstr(&ancestor.id, "d427e0b2e138501a3d15cc376077a3631e15bd46"); + git_oid_fromstr(&ours.id, "ee3fa1b8c00aff7fe02065fdb50864bb0d932ccf"); + git_oid_fromstr(&theirs.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863"); cl_git_pass(git_index_conflict_add(index, &ancestor, &ours, &theirs)); cl_git_pass(git_diff_tree_to_index(&diff, g_repo, a, index, &opts)); @@ -255,7 +255,7 @@ void test_diff_index__not_in_head_conflicted(void) theirs.path = "file_not_in_head"; theirs.mode = GIT_FILEMODE_BLOB; - git_oid_fromstr(&theirs.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + git_oid_fromstr(&theirs.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863"); cl_git_pass(git_index_conflict_add(index, NULL, NULL, &theirs)); cl_git_pass(git_diff_tree_to_index(&diff, g_repo, a, index, NULL)); diff --git a/tests/diff/workdir.c b/tests/diff/workdir.c index 892c7b72d..e1bbce8fb 100644 --- a/tests/diff/workdir.c +++ b/tests/diff/workdir.c @@ -85,9 +85,11 @@ void test_diff_workdir__to_index_with_conflicts(void) /* Adding an entry that represents a rename gets two files in conflict */ our_entry.path = "subdir/modified_file"; our_entry.mode = 0100644; + git_oid_fromstr(&our_entry.id, "ee3fa1b8c00aff7fe02065fdb50864bb0d932ccf"); their_entry.path = "subdir/rename_conflict"; their_entry.mode = 0100644; + git_oid_fromstr(&their_entry.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863"); cl_git_pass(git_repository_index(&index, g_repo)); cl_git_pass(git_index_conflict_add(index, NULL, &our_entry, &their_entry)); @@ -1975,9 +1977,9 @@ void test_diff_workdir__to_index_conflicted(void) { ancestor.path = ours.path = theirs.path = "_file"; ancestor.mode = ours.mode = theirs.mode = 0100644; - git_oid_fromstr(&ancestor.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); - git_oid_fromstr(&ours.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); - git_oid_fromstr(&theirs.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + git_oid_fromstr(&ancestor.id, "d427e0b2e138501a3d15cc376077a3631e15bd46"); + git_oid_fromstr(&ours.id, "ee3fa1b8c00aff7fe02065fdb50864bb0d932ccf"); + git_oid_fromstr(&theirs.id, "2bd0a343aeef7a2cf0d158478966a6e587ff3863"); cl_git_pass(git_index_conflict_add(index, &ancestor, &ours, &theirs)); cl_git_pass(git_diff_tree_to_index(&diff1, g_repo, a, index, NULL)); diff --git a/tests/index/bypath.c b/tests/index/bypath.c index 88a76178a..34a7412a8 100644 --- a/tests/index/bypath.c +++ b/tests/index/bypath.c @@ -134,6 +134,7 @@ void test_index_bypath__add_honors_existing_case_2(void) clar__skip(); dummy.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_oid_fromstr(&dummy.id, "f990a25a74d1a8281ce2ab018ea8df66795cd60b")); /* note that `git_index_add` does no checking to canonical directories */ dummy.path = "Just_a_dir/file0.txt"; @@ -189,6 +190,7 @@ void test_index_bypath__add_honors_existing_case_3(void) clar__skip(); dummy.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_oid_fromstr(&dummy.id, "f990a25a74d1a8281ce2ab018ea8df66795cd60b")); dummy.path = "just_a_dir/filea.txt"; cl_git_pass(git_index_add(g_idx, &dummy)); @@ -219,6 +221,7 @@ void test_index_bypath__add_honors_existing_case_4(void) clar__skip(); dummy.mode = GIT_FILEMODE_BLOB; + cl_git_pass(git_oid_fromstr(&dummy.id, "f990a25a74d1a8281ce2ab018ea8df66795cd60b")); dummy.path = "just_a_dir/a/b/c/d/e/file1.txt"; cl_git_pass(git_index_add(g_idx, &dummy)); diff --git a/tests/index/cache.c b/tests/index/cache.c index 3982bf183..56885aff7 100644 --- a/tests/index/cache.c +++ b/tests/index/cache.c @@ -111,7 +111,7 @@ void test_index_cache__read_tree_no_children(void) memset(&entry, 0x0, sizeof(git_index_entry)); entry.path = "new.txt"; entry.mode = GIT_FILEMODE_BLOB; - git_oid_fromstr(&entry.id, "45b983be36b73c0788dc9cbcb76cbb80fc7bb057"); + git_oid_fromstr(&entry.id, "d4bcc68acd4410bf836a39f20afb2c2ece09584e"); cl_git_pass(git_index_add(index, &entry)); cl_assert_equal_i(-1, index->tree->entry_count); @@ -191,7 +191,7 @@ void test_index_cache__read_tree_children(void) memset(&entry, 0x0, sizeof(git_index_entry)); entry.path = "top-level"; entry.mode = GIT_FILEMODE_BLOB; - git_oid_fromstr(&entry.id, "45b983be36b73c0788dc9cbcb76cbb80fc7bb057"); + git_oid_fromstr(&entry.id, "ee3fa1b8c00aff7fe02065fdb50864bb0d932ccf"); cl_git_pass(git_index_add(index, &entry)); @@ -217,7 +217,7 @@ void test_index_cache__read_tree_children(void) /* override with a slightly different id, also dummy */ entry.path = "subdir/some-file"; - git_oid_fromstr(&entry.id, "45b983be36b73c0788dc9cbcb76cbb80fc7bb058"); + git_oid_fromstr(&entry.id, "ee3fa1b8c00aff7fe02065fdb50864bb0d932ccf"); cl_git_pass(git_index_add(index, &entry)); cl_assert_equal_i(-1, index->tree->entry_count); diff --git a/tests/index/conflicts.c b/tests/index/conflicts.c index 8e94cd441..d4004686f 100644 --- a/tests/index/conflicts.c +++ b/tests/index/conflicts.c @@ -16,11 +16,6 @@ static git_index *repo_index; #define CONFLICTS_TWO_OUR_OID "8b3f43d2402825c200f835ca1762413e386fd0b2" #define CONFLICTS_TWO_THEIR_OID "220bd62631c8cf7a83ef39c6b94595f00517211e" -#define TEST_STAGED_OID "beefdadafeedabedcafedeedbabedeadbeaddeaf" -#define TEST_ANCESTOR_OID "f00ff00ff00ff00ff00ff00ff00ff00ff00ff00f" -#define TEST_OUR_OID "b44bb44bb44bb44bb44bb44bb44bb44bb44bb44b" -#define TEST_THEIR_OID "0123456789abcdef0123456789abcdef01234567" - // Fixture setup and teardown void test_index_conflicts__initialize(void) { @@ -49,17 +44,17 @@ void test_index_conflicts__add(void) ancestor_entry.path = "test-one.txt"; ancestor_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&ancestor_entry, 1); - git_oid_fromstr(&ancestor_entry.id, TEST_ANCESTOR_OID); + git_oid_fromstr(&ancestor_entry.id, CONFLICTS_ONE_ANCESTOR_OID); our_entry.path = "test-one.txt"; our_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&our_entry, 2); - git_oid_fromstr(&our_entry.id, TEST_OUR_OID); + git_oid_fromstr(&our_entry.id, CONFLICTS_ONE_OUR_OID); their_entry.path = "test-one.txt"; their_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&ancestor_entry, 2); - git_oid_fromstr(&their_entry.id, TEST_THEIR_OID); + git_oid_fromstr(&their_entry.id, CONFLICTS_ONE_THEIR_OID); cl_git_pass(git_index_conflict_add(repo_index, &ancestor_entry, &our_entry, &their_entry)); @@ -80,17 +75,17 @@ void test_index_conflicts__add_fixes_incorrect_stage(void) ancestor_entry.path = "test-one.txt"; ancestor_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&ancestor_entry, 3); - git_oid_fromstr(&ancestor_entry.id, TEST_ANCESTOR_OID); + git_oid_fromstr(&ancestor_entry.id, CONFLICTS_ONE_ANCESTOR_OID); our_entry.path = "test-one.txt"; our_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&our_entry, 1); - git_oid_fromstr(&our_entry.id, TEST_OUR_OID); + git_oid_fromstr(&our_entry.id, CONFLICTS_ONE_OUR_OID); their_entry.path = "test-one.txt"; their_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&their_entry, 2); - git_oid_fromstr(&their_entry.id, TEST_THEIR_OID); + git_oid_fromstr(&their_entry.id, CONFLICTS_ONE_THEIR_OID); cl_git_pass(git_index_conflict_add(repo_index, &ancestor_entry, &our_entry, &their_entry)); @@ -105,36 +100,33 @@ void test_index_conflicts__add_fixes_incorrect_stage(void) void test_index_conflicts__add_removes_stage_zero(void) { - git_index_entry staged, ancestor_entry, our_entry, their_entry; + git_index_entry ancestor_entry, our_entry, their_entry; const git_index_entry *conflict_entry[3]; cl_assert(git_index_entrycount(repo_index) == 8); - memset(&staged, 0x0, sizeof(git_index_entry)); memset(&ancestor_entry, 0x0, sizeof(git_index_entry)); memset(&our_entry, 0x0, sizeof(git_index_entry)); memset(&their_entry, 0x0, sizeof(git_index_entry)); - staged.path = "test-one.txt"; - staged.mode = 0100644; - git_oid_fromstr(&staged.id, TEST_STAGED_OID); - cl_git_pass(git_index_add(repo_index, &staged)); + cl_git_mkfile("./mergedrepo/test-one.txt", "new-file\n"); + cl_git_pass(git_index_add_bypath(repo_index, "test-one.txt")); cl_assert(git_index_entrycount(repo_index) == 9); ancestor_entry.path = "test-one.txt"; ancestor_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&ancestor_entry, 3); - git_oid_fromstr(&ancestor_entry.id, TEST_ANCESTOR_OID); + git_oid_fromstr(&ancestor_entry.id, CONFLICTS_ONE_ANCESTOR_OID); our_entry.path = "test-one.txt"; our_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&our_entry, 1); - git_oid_fromstr(&our_entry.id, TEST_OUR_OID); + git_oid_fromstr(&our_entry.id, CONFLICTS_ONE_OUR_OID); their_entry.path = "test-one.txt"; their_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&their_entry, 2); - git_oid_fromstr(&their_entry.id, TEST_THEIR_OID); + git_oid_fromstr(&their_entry.id, CONFLICTS_ONE_THEIR_OID); cl_git_pass(git_index_conflict_add(repo_index, &ancestor_entry, &our_entry, &their_entry)); @@ -330,7 +322,7 @@ void test_index_conflicts__partial(void) ancestor_entry.path = "test-one.txt"; ancestor_entry.mode = 0100644; GIT_IDXENTRY_STAGE_SET(&ancestor_entry, 1); - git_oid_fromstr(&ancestor_entry.id, TEST_ANCESTOR_OID); + git_oid_fromstr(&ancestor_entry.id, CONFLICTS_ONE_ANCESTOR_OID); cl_git_pass(git_index_conflict_add(repo_index, &ancestor_entry, NULL, NULL)); cl_assert(git_index_entrycount(repo_index) == 9); diff --git a/tests/index/filemodes.c b/tests/index/filemodes.c index 6442d7755..2efad5b33 100644 --- a/tests/index/filemodes.c +++ b/tests/index/filemodes.c @@ -236,12 +236,19 @@ void test_index_filemodes__invalid(void) { git_index *index; git_index_entry entry; + const git_index_entry *dummy; cl_git_pass(git_repository_index(&index, g_repo)); + /* add a dummy file so that we have a valid id */ + cl_git_mkfile("./filemodes/dummy-file.txt", "new-file\n"); + cl_git_pass(git_index_add_bypath(index, "dummy-file.txt")); + cl_assert((dummy = git_index_get_bypath(index, "dummy-file.txt", 0))); + GIT_IDXENTRY_STAGE_SET(&entry, 0); entry.path = "foo"; entry.mode = GIT_OBJ_BLOB; + git_oid_cpy(&entry.id, &dummy->id); cl_git_fail(git_index_add(index, &entry)); entry.mode = GIT_FILEMODE_BLOB; diff --git a/tests/index/racy.c b/tests/index/racy.c index ace84d585..68aa46007 100644 --- a/tests/index/racy.c +++ b/tests/index/racy.c @@ -178,6 +178,7 @@ static void setup_uptodate_files(void) { git_buf path = GIT_BUF_INIT; git_index *index; + const git_index_entry *a_entry; git_index_entry new_entry = {{0}}; cl_git_pass(git_repository_index(&index, g_repo)); @@ -188,9 +189,12 @@ static void setup_uptodate_files(void) /* Put 'A' into the index */ cl_git_pass(git_index_add_bypath(index, "A")); + cl_assert((a_entry = git_index_get_bypath(index, "A", 0))); + /* Put 'B' into the index */ new_entry.path = "B"; new_entry.mode = GIT_FILEMODE_BLOB; + git_oid_cpy(&new_entry.id, &a_entry->id); cl_git_pass(git_index_add(index, &new_entry)); /* Put 'C' into the index */ diff --git a/tests/object/tree/write.c b/tests/object/tree/write.c index 4cd1607d8..f779b8ce6 100644 --- a/tests/object/tree/write.c +++ b/tests/object/tree/write.c @@ -133,15 +133,18 @@ void test_object_tree_write__sorted_subtrees(void) { GIT_FILEMODE_TREE, "vendors"} }; - git_oid blank_oid, tree_oid; + git_oid bid, tid, tree_oid; - memset(&blank_oid, 0x0, sizeof(blank_oid)); + cl_git_pass(git_oid_fromstr(&bid, blob_oid)); + cl_git_pass(git_oid_fromstr(&tid, first_tree)); cl_git_pass(git_treebuilder_new(&builder, g_repo, NULL)); for (i = 0; i < ARRAY_SIZE(entries); ++i) { + git_oid *id = entries[i].attr == GIT_FILEMODE_TREE ? &tid : &bid; + cl_git_pass(git_treebuilder_insert(NULL, - builder, entries[i].filename, &blank_oid, entries[i].attr)); + builder, entries[i].filename, id, entries[i].attr)); } cl_git_pass(git_treebuilder_write(&tree_oid, builder)); @@ -189,10 +192,10 @@ void test_object_tree_write__removing_and_re_adding_in_treebuilder(void) { git_treebuilder *builder; int i, aardvark_i, apple_i, apple_after_i, apple_extra_i, last_i; - git_oid blank_oid, tree_oid; + git_oid entry_oid, tree_oid; git_tree *tree; - memset(&blank_oid, 0x0, sizeof(blank_oid)); + cl_git_pass(git_oid_fromstr(&entry_oid, blob_oid)); cl_git_pass(git_treebuilder_new(&builder, g_repo, NULL)); @@ -200,7 +203,7 @@ void test_object_tree_write__removing_and_re_adding_in_treebuilder(void) for (i = 0; _entries[i].filename; ++i) cl_git_pass(git_treebuilder_insert(NULL, - builder, _entries[i].filename, &blank_oid, _entries[i].attr)); + builder, _entries[i].filename, &entry_oid, _entries[i].attr)); cl_assert_equal_i(6, (int)git_treebuilder_entrycount(builder)); @@ -211,12 +214,12 @@ void test_object_tree_write__removing_and_re_adding_in_treebuilder(void) cl_assert_equal_i(4, (int)git_treebuilder_entrycount(builder)); cl_git_pass(git_treebuilder_insert( - NULL, builder, "before_last", &blank_oid, GIT_FILEMODE_BLOB)); + NULL, builder, "before_last", &entry_oid, GIT_FILEMODE_BLOB)); cl_assert_equal_i(5, (int)git_treebuilder_entrycount(builder)); /* reinsert apple_after */ cl_git_pass(git_treebuilder_insert( - NULL, builder, "apple_after", &blank_oid, GIT_FILEMODE_BLOB)); + NULL, builder, "apple_after", &entry_oid, GIT_FILEMODE_BLOB)); cl_assert_equal_i(6, (int)git_treebuilder_entrycount(builder)); cl_git_pass(git_treebuilder_remove(builder, "last")); @@ -224,11 +227,11 @@ void test_object_tree_write__removing_and_re_adding_in_treebuilder(void) /* reinsert last */ cl_git_pass(git_treebuilder_insert( - NULL, builder, "last", &blank_oid, GIT_FILEMODE_BLOB)); + NULL, builder, "last", &entry_oid, GIT_FILEMODE_BLOB)); cl_assert_equal_i(6, (int)git_treebuilder_entrycount(builder)); cl_git_pass(git_treebuilder_insert( - NULL, builder, "apple_extra", &blank_oid, GIT_FILEMODE_BLOB)); + NULL, builder, "apple_extra", &entry_oid, GIT_FILEMODE_BLOB)); cl_assert_equal_i(7, (int)git_treebuilder_entrycount(builder)); cl_git_pass(git_treebuilder_write(&tree_oid, builder)); @@ -280,16 +283,16 @@ void test_object_tree_write__filtering(void) { git_treebuilder *builder; int i; - git_oid blank_oid, tree_oid; + git_oid entry_oid, tree_oid; git_tree *tree; - memset(&blank_oid, 0x0, sizeof(blank_oid)); + git_oid_fromstr(&entry_oid, blob_oid); cl_git_pass(git_treebuilder_new(&builder, g_repo, NULL)); for (i = 0; _entries[i].filename; ++i) cl_git_pass(git_treebuilder_insert(NULL, - builder, _entries[i].filename, &blank_oid, _entries[i].attr)); + builder, _entries[i].filename, &entry_oid, _entries[i].attr)); cl_assert_equal_i(6, (int)git_treebuilder_entrycount(builder)); @@ -408,6 +411,8 @@ void test_object_tree_write__protect_filesystems(void) git_treebuilder *builder; git_oid bid; + cl_git_pass(git_oid_fromstr(&bid, "fa49b077972391ad58037050f2a75f74e3671e92")); + /* Ensure that (by default) we can write objects with funny names on * platforms that are not affected. */ diff --git a/tests/resources/status/.gitted/objects/2b/d0a343aeef7a2cf0d158478966a6e587ff3863 b/tests/resources/status/.gitted/objects/2b/d0a343aeef7a2cf0d158478966a6e587ff3863 new file mode 100644 index 0000000000000000000000000000000000000000..d10ca636b6bf5f66bce633362d871d17d9a292c6 GIT binary patch literal 56 zcmV-80LTA$0ZYosPf{>3VkpVTELKR%%t=)M(#aW#dFiPs3YmEdNkxfy$r%cXc_|9H OiNz(UMO*-@y%7?c&=$P_ literal 0 HcmV?d00001 diff --git a/tests/resources/status/.gitted/objects/d4/27e0b2e138501a3d15cc376077a3631e15bd46 b/tests/resources/status/.gitted/objects/d4/27e0b2e138501a3d15cc376077a3631e15bd46 new file mode 100644 index 0000000000000000000000000000000000000000..0b3611ae4c9dc635dafd5dc9560cbc2ff5e92198 GIT binary patch literal 38 ucmb4F=r^r$ShV!%gjkt0Mf}BiFxU%DGHf+3b~2JC84F=r^r$ShV!%gjkt0Mf}BiFxU%DGHf+3b~2JC8 Date: Sun, 28 Feb 2016 15:51:38 -0500 Subject: [PATCH 9/9] turn on strict object validation by default --- CHANGELOG.md | 5 +++++ src/object.c | 2 +- tests/commit/write.c | 8 +++---- tests/index/add.c | 46 +++++++++++++++++++-------------------- tests/object/tree/write.c | 12 +++++----- 5 files changed, 39 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec5a0d336..9c50f1211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,11 @@ v0.23 + 1 * Rebases can now be performed purely in-memory, without touching the repository's workdir. +* When adding objects to the index, or when creating new tree or commit + objects, the inputs are validated to ensure that the dependent objects + exist and are of the correct type. This object validation can be + disabled with the GIT_OPT_ENABLE_STRICT_OBJECT_CREATION option. + ### API additions * `git_config_lock()` has been added, which allow for diff --git a/src/object.c b/src/object.c index e7c1fef09..ebf77fb47 100644 --- a/src/object.c +++ b/src/object.c @@ -14,7 +14,7 @@ #include "blob.h" #include "tag.h" -bool git_object__strict_input_validation = false; +bool git_object__strict_input_validation = true; typedef struct { const char *str; /* type name string */ diff --git a/tests/commit/write.c b/tests/commit/write.c index 303d1ce58..96b7cc321 100644 --- a/tests/commit/write.c +++ b/tests/commit/write.c @@ -38,7 +38,7 @@ void test_commit_write__cleanup(void) cl_git_sandbox_cleanup(); - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); } @@ -196,10 +196,12 @@ static int create_commit_from_ids( return ret; } -void test_commit_write__doesnt_validate_objects_by_default(void) +void test_commit_write__can_write_invalid_objects(void) { git_oid expected_id, tree_id, parent_id, commit_id; + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + /* this is a valid tree and parent */ git_oid_fromstr(&tree_id, tree_id_str); git_oid_fromstr(&parent_id, parent_id_str); @@ -237,8 +239,6 @@ void test_commit_write__can_validate_objects(void) { git_oid tree_id, parent_id, commit_id; - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); - /* this is a valid tree and parent */ git_oid_fromstr(&tree_id, tree_id_str); git_oid_fromstr(&parent_id, parent_id_str); diff --git a/tests/index/add.c b/tests/index/add.c index cfa81c4d9..f101ea266 100644 --- a/tests/index/add.c +++ b/tests/index/add.c @@ -20,7 +20,7 @@ void test_index_add__cleanup(void) cl_git_sandbox_cleanup(); g_repo = NULL; - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); } static void test_add_entry( @@ -42,7 +42,7 @@ static void test_add_entry( void test_index_add__invalid_entries_succeeds_by_default(void) { /* - * Ensure that there is no validation on ids by default + * Ensure that there is validation on object ids by default */ /* ensure that we can add some actually good entries */ @@ -50,27 +50,6 @@ void test_index_add__invalid_entries_succeeds_by_default(void) 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); @@ -80,5 +59,26 @@ void test_index_add__invalid_entries_succeeds_by_default(void) 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); + + /* + * Ensure that there we can disable validation + */ + + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + + /* 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 can now 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 do not 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); } diff --git a/tests/object/tree/write.c b/tests/object/tree/write.c index f779b8ce6..341f5db72 100644 --- a/tests/object/tree/write.c +++ b/tests/object/tree/write.c @@ -19,7 +19,7 @@ void test_object_tree_write__cleanup(void) { cl_git_sandbox_cleanup(); - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); } void test_object_tree_write__from_memory(void) @@ -492,11 +492,11 @@ static void test_invalid_objects(bool should_allow_invalid) void test_object_tree_write__object_validity(void) { - /* Ensure that we can add invalid objects by default */ - test_invalid_objects(true); - - /* Ensure that we can turn on validation */ - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); + /* Ensure that we cannot add invalid objects by default */ test_invalid_objects(false); + + /* Ensure that we can turn off validation */ + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + test_invalid_objects(true); }