Commit 335c9e2f45b75e9fe8d8256e6ecfdb3d333fcbcc

Stjepan Rajko 2015-10-26T15:33:00

Prevent segfault when parsing a reflog with oid parse error Using calloc instead of malloc because the parse error will lead to an immediate free of committer (and its properties, which can segfault on free if undefined - test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_returns_error segfaulted before the fix). #3458

diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index 921f786..9f87b30 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1463,7 +1463,7 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size)
 		entry = git__calloc(1, sizeof(git_reflog_entry));
 		GITERR_CHECK_ALLOC(entry);
 
-		entry->committer = git__malloc(sizeof(git_signature));
+		entry->committer = git__calloc(1, sizeof(git_signature));
 		GITERR_CHECK_ALLOC(entry->committer);
 
 		if (git_oid_fromstrn(&entry->oid_old, buf, GIT_OID_HEXSZ) < 0)
diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c
index 56ec422..3fbf412 100644
--- a/tests/refs/reflog/reflog.c
+++ b/tests/refs/reflog/reflog.c
@@ -154,6 +154,49 @@ void test_refs_reflog_reflog__reading_the_reflog_from_a_reference_with_no_log_re
 	git_buf_free(&subtrees_log_path);
 }
 
+void test_refs_reflog_reflog__reading_a_reflog_with_invalid_format_returns_error(void)
+{
+	git_reflog *reflog;
+	const git_error *error;
+	const char *refname = "refs/heads/newline";
+	const char *refmessage =
+		"Reflog*message with a newline and enough content after it to pass the GIT_REFLOG_SIZE_MIN check inside reflog_parse.";
+	git_reference *ref;
+	git_oid id;
+	git_buf logpath = GIT_BUF_INIT, logcontents = GIT_BUF_INIT;
+	char *star;
+
+	git_oid_fromstr(&id, current_master_tip);
+
+	/* create a new branch */
+	cl_git_pass(git_reference_create(&ref, g_repo, refname, &id, 1, refmessage));
+
+	/* corrupt the branch reflog by introducing a newline inside the reflog message (we replace '*' with '\n') */
+	git_buf_join_n(&logpath, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, refname);
+	cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath)));
+	cl_assert((star = strchr(git_buf_cstr(&logcontents), '*')) != NULL);
+	*star = '\n';
+	cl_git_rewritefile(git_buf_cstr(&logpath), git_buf_cstr(&logcontents));
+
+	/* confirm that the file was rewritten successfully and now contains a '\n' in the expected location */
+	cl_git_pass(git_futils_readbuffer(&logcontents, git_buf_cstr(&logpath)));
+	cl_assert(strstr(git_buf_cstr(&logcontents), "Reflog\nmessage") != NULL);
+
+	/* clear the error state so we can capture the error generated by git_reflog_read */
+	giterr_clear();
+
+	cl_git_fail(git_reflog_read(&reflog, g_repo, refname));
+
+	error = giterr_last();
+
+	cl_assert(error != NULL);
+	cl_assert_equal_s("Unable to parse OID - contains invalid characters", error->message);
+
+	git_reference_free(ref);
+	git_buf_free(&logpath);
+	git_buf_free(&logcontents);
+}
+
 void test_refs_reflog_reflog__cannot_write_a_moved_reflog(void)
 {
 	git_reference *master, *new_master;