commit: safer commit creation with reference update

The current version of the commit creation and amend function are unsafe
to use when passing the update_ref parameter, as they do not check that
the reference at the moment of update points to what the user expects.

Make sure that we're moving history forward when we ask the library to
update the reference for us by checking that the first parent of the new
commit is the current value of the reference. We also make sure that the
ref we're updating hasn't moved between the read and the write.

Similarly, when amending a commit, make sure that the current tip of the
branch is the commit we're amending.

1 files changed, 78 insertions, 21 deletions

diff --git a/src/commit.c b/src/commit.c
index 255debe82..227d5c4a5 100644
--- a/src/commit.c
+++ b/src/commit.c
@@ -34,6 +34,35 @@ void git_commit__free(void *_commit)
 	git__free(commit);
 }
 
+static int update_ref_for_commit(git_repository *repo, git_reference *ref, const char *update_ref, const git_oid *id, const git_signature *committer)
+{
+	git_reference *ref2 = NULL;
+	int error;
+	git_commit *c;
+	const char *shortmsg;
+	git_buf reflog_msg = GIT_BUF_INIT;
+
+	if ((error = git_commit_lookup(&c, repo, id)) < 0) {
+		return error;
+	}
+
+	shortmsg = git_commit_summary(c);
+	git_buf_printf(&reflog_msg, "commit%s: %s",
+		       git_commit_parentcount(c) == 0 ? " (initial)" : "",
+		       shortmsg);
+	git_commit_free(c);
+
+	if (ref) {
+		error = git_reference_set_target(&ref2, ref, id, committer, git_buf_cstr(&reflog_msg));
+		git_reference_free(ref2);
+	} else {
+		error = git_reference__update_terminal(repo, update_ref, id, committer, git_buf_cstr(&reflog_msg));
+	}
+
+	git_buf_free(&reflog_msg);
+	return error;
+}
+
 int git_commit_create_from_callback(
 	git_oid *id,
 	git_repository *repo,
@@ -46,6 +75,9 @@ int git_commit_create_from_callback(
 	git_commit_parent_callback parent_cb,
 	void *parent_payload)
 {
+	git_reference *ref = NULL;
+	int error = 0, matched_parent = 0;
+	const git_oid *current_id = NULL;
 	git_buf commit = GIT_BUF_INIT;
 	size_t i = 0;
 	git_odb *odb;
@@ -53,10 +85,31 @@ int git_commit_create_from_callback(
 
 	assert(id && repo && tree && parent_cb);
 
+	if (update_ref) {
+		error = git_reference_lookup_resolved(&ref, repo, update_ref, 10);
+		if (error < 0 && error != GIT_ENOTFOUND)
+			return error;
+	}
+	giterr_clear();
+
+	if (ref)
+		current_id = git_reference_target(ref);
+
 	git_oid__writebuf(&commit, "tree ", tree);
 
-	while ((parent = parent_cb(i++, parent_payload)) != NULL)
+	while ((parent = parent_cb(i, parent_payload)) != NULL) {
 		git_oid__writebuf(&commit, "parent ", parent);
+		if (i == 0 && current_id && git_oid_equal(current_id, parent))
+			matched_parent = 1;
+		i++;
+	}
+
+	if (ref && !matched_parent) {
+		git_reference_free(ref);
+		git_buf_free(&commit);
+		giterr_set(GITERR_OBJECT, "failed to create commit: current tip is not the first parent");
+		return GIT_EMODIFIED;
+	}
 
 	git_signature__writebuf(&commit, "author ", author);
 	git_signature__writebuf(&commit, "committer ", committer);
@@ -78,24 +131,8 @@ int git_commit_create_from_callback(
 	git_buf_free(&commit);
 
 	if (update_ref != NULL) {
-		int error;
-		git_commit *c;
-		const char *shortmsg;
-		git_buf reflog_msg = GIT_BUF_INIT;
-
-		if (git_commit_lookup(&c, repo, id) < 0)
-			goto on_error;
-
-		shortmsg = git_commit_summary(c);
-		git_buf_printf(&reflog_msg, "commit%s: %s",
-				git_commit_parentcount(c) == 0 ? " (initial)" : "",
-				shortmsg);
-		git_commit_free(c);
-
-		error = git_reference__update_terminal(repo, update_ref, id,
-				committer, git_buf_cstr(&reflog_msg));
-
-		git_buf_free(&reflog_msg);
+		error = update_ref_for_commit(repo, ref, update_ref, id, committer);
+		git_reference_free(ref);
 		return error;
 	}
 
@@ -242,6 +279,8 @@ int git_commit_amend(
 {
 	git_repository *repo;
 	git_oid tree_id;
+	git_reference *ref;
+	int error;
 
 	assert(id && commit_to_amend);
 
@@ -266,9 +305,27 @@ int git_commit_amend(
 		git_oid_cpy(&tree_id, git_tree_id(tree));
 	}
 
-	return git_commit_create_from_callback(
-		id, repo, update_ref, author, committer, message_encoding, message,
+	if (update_ref) {
+		if ((error = git_reference_lookup_resolved(&ref, repo, update_ref, 5)) < 0)
+			return error;
+
+		if (git_oid_cmp(git_commit_id(commit_to_amend), git_reference_target(ref))) {
+			git_reference_free(ref);
+			giterr_set(GITERR_REFERENCE, "commit to amend is not the tip of the given branch");
+			return -1;
+		}
+	}
+
+	error = git_commit_create_from_callback(
+		id, repo, NULL, author, committer, message_encoding, message,
 		&tree_id, commit_parent_for_amend, (void *)commit_to_amend);
+
+	if (!error && update_ref) {
+		error = update_ref_for_commit(repo, ref, NULL, id, committer);
+		git_reference_free(ref);
+	}
+
+	return error;
 }
 
 int git_commit__parse(void *_commit, git_odb_object *odb_obj)