Merge pull request #7239 from libgit2/ethomson/mergefix

merge_files: avoid UB in xdiff

deps/xdiff/xmerge.c

@@ -709,19 +709,23 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
 		goto out;
 
 	if (!xscr1) {
-		result->ptr = xdl_malloc(mf2->size);
-		if (!result->ptr)
-			goto out;
+		if (mf2->size) {
+			result->ptr = xdl_malloc(mf2->size);
+			if (!result->ptr)
+				goto out;
+			memcpy(result->ptr, mf2->ptr, mf2->size);
+			result->size = mf2->size;
+		}
 		status = 0;
-		memcpy(result->ptr, mf2->ptr, mf2->size);
-		result->size = mf2->size;
 	} else if (!xscr2) {
-		result->ptr = xdl_malloc(mf1->size);
-		if (!result->ptr)
-			goto out;
+		if (mf1->size) {
+			result->ptr = xdl_malloc(mf1->size);
+			if (!result->ptr)
+				goto out;
+			memcpy(result->ptr, mf1->ptr, mf1->size);
+			result->size = mf1->size;
+		}
 		status = 0;
-		memcpy(result->ptr, mf1->ptr, mf1->size);
-		result->size = mf1->size;
 	} else {
 		status = xdl_do_merge(&xe1, xscr1,
 				      &xe2, xscr2,

include/git2/merge.h

@@ -568,8 +568,10 @@ GIT_EXTERN(int) git_merge_file(
 /**
  * Merge two files as they exist in the index, using the given common
  * ancestor as the baseline, producing a `git_merge_file_result` that
- * reflects the merge result.  The `git_merge_file_result` must be freed with
- * `git_merge_file_result_free`.
+ * reflects the merge result.  The `git_merge_file_result` must be
+ * freed with `git_merge_file_result_free`.
+ *
+ * At least one of `ancestor`, `ours`, or `theirs` must be non-null.
  *
  * @param out The git_merge_file_result to be filled in
  * @param repo The repository

src/libgit2/merge_file.c

@@ -92,21 +92,21 @@ static int merge_file__xdiff(
 		goto done;
 	}
 
-	if (ancestor) {
+	if (ancestor && ancestor->size) {
 		xmparam.ancestor = (options.ancestor_label) ?
 			options.ancestor_label : ancestor->path;
 		ancestor_mmfile.ptr = (char *)ancestor->ptr;
 		ancestor_mmfile.size = (long)ancestor->size;
 	}
 
-	if (ours) {
+	if (ours && ours->size) {
 		xmparam.file1 = (options.our_label) ?
 			options.our_label : ours->path;
 		our_mmfile.ptr = (char *)ours->ptr;
 		our_mmfile.size = (long)ours->size;
 	}
 
-	if (theirs) {
+	if (theirs && theirs->size) {
 		xmparam.file2 = (options.their_label) ?
 			options.their_label : theirs->path;
 		their_mmfile.ptr = (char *)theirs->ptr;
@@ -205,7 +205,7 @@ static int merge_file__binary(
 		goto done;
 
 	if ((out->path = git__strdup(favored->path)) == NULL ||
-		(out->ptr = git__malloc(favored->size)) == NULL)
+	    (out->ptr = git__malloc(favored->size)) == NULL)
 		goto done;
 
 	memcpy((char *)out->ptr, favored->ptr, favored->size);
@@ -225,8 +225,8 @@ static int merge_file__from_inputs(
 	const git_merge_file_options *given_opts)
 {
 	if (merge_file__is_binary(ancestor) ||
-		merge_file__is_binary(ours) ||
-		merge_file__is_binary(theirs))
+	    merge_file__is_binary(ours) ||
+	    merge_file__is_binary(theirs))
 		return merge_file__binary(out, ours, theirs, given_opts);
 
 	return merge_file__xdiff(out, ancestor, ours, theirs, given_opts);
@@ -265,8 +265,11 @@ int git_merge_file(
 	if (ancestor)
 		ancestor = git_merge_file__normalize_inputs(&inputs[0], ancestor);
 
-	ours = git_merge_file__normalize_inputs(&inputs[1], ours);
-	theirs = git_merge_file__normalize_inputs(&inputs[2], theirs);
+	if (ours)
+		ours = git_merge_file__normalize_inputs(&inputs[1], ours);
+
+	if (theirs)
+		theirs = git_merge_file__normalize_inputs(&inputs[2], theirs);
 
 	return merge_file__from_inputs(out, ancestor, ours, theirs, options);
 }
@@ -279,18 +282,16 @@ int git_merge_file_from_index(
 	const git_index_entry *theirs,
 	const git_merge_file_options *options)
 {
-	git_merge_file_input *ancestor_ptr = NULL,
-		ancestor_input = {0};
-	git_merge_file_input *our_ptr = NULL,
-		our_input = {0};
-	git_merge_file_input *their_ptr = NULL,
-		their_input = {0};
+	git_merge_file_input ancestor_input = {0};
+	git_merge_file_input our_input = {0};
+	git_merge_file_input their_input = {0};
 	git_odb *odb;
 	git_odb_object *odb_object[3] = { 0 };
 	int error = 0;
 
 	GIT_ASSERT_ARG(out);
 	GIT_ASSERT_ARG(repo);
+	GIT_ASSERT_ARG(ancestor || ours || theirs);
 
 	memset(out, 0x0, sizeof(git_merge_file_result));
 
@@ -301,28 +302,22 @@ int git_merge_file_from_index(
 		if ((error = merge_file_input_from_index(
 			&ancestor_input, &odb_object[0], odb, ancestor)) < 0)
 			goto done;
-
-		ancestor_ptr = &ancestor_input;
 	}
 
 	if (ours) {
 		if ((error = merge_file_input_from_index(
 			&our_input, &odb_object[1], odb, ours)) < 0)
 			goto done;
-
-		our_ptr = &our_input;
 	}
 
 	if (theirs) {
 		if ((error = merge_file_input_from_index(
 			&their_input, &odb_object[2], odb, theirs)) < 0)
 			goto done;
-
-		their_ptr = &their_input;
 	}
 
 	error = merge_file__from_inputs(out,
-		ancestor_ptr, our_ptr, their_ptr, options);
+		&ancestor_input, &our_input, &their_input, options);
 
 done:
 	git_odb_object_free(odb_object[0]);

tests/libgit2/merge/files.c

@@ -175,6 +175,36 @@ void test_merge_files__automerge_from_index(void)
 	git_merge_file_result_free(&result);
 }
 
+void test_merge_files__automerge_zero_byte(void)
+{
+	git_merge_file_result result = {0};
+	git_index_entry ancestor, theirs, ours;
+
+	git_oid_from_string(&ancestor.id, "d427e0b2e138501a3d15cc376077a3631e15bd46", GIT_OID_SHA1);
+	ancestor.path = "automergeable.txt";
+	ancestor.mode = GIT_FILEMODE_BLOB;
+
+	git_oid_from_string(&ours.id, "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391", GIT_OID_SHA1);
+	ours.path = "empty.txt";
+	ours.mode = GIT_FILEMODE_BLOB;
+
+	git_oid_from_string(&theirs.id, "d427e0b2e138501a3d15cc376077a3631e15bd46", GIT_OID_SHA1);
+	theirs.path = "automergeable.txt";
+	theirs.mode = GIT_FILEMODE_BLOB;
+
+	cl_git_pass(git_merge_file_from_index(&result, repo,
+		&ancestor, &ours, &theirs, 0));
+
+	cl_assert_equal_i(1, result.automergeable);
+
+	cl_assert_equal_s("empty.txt", result.path);
+	cl_assert_equal_i(GIT_FILEMODE_BLOB, result.mode);
+
+	cl_assert_equal_i(0, result.len);
+
+	git_merge_file_result_free(&result);
+}
+
 void test_merge_files__automerge_from_index_delete_file(void)
 {
 	git_merge_file_result result = {0};
@@ -204,7 +234,7 @@ void test_merge_files__automerge_from_index_delete_file(void)
 void test_merge_files__automerge_from_index_add_file(void)
 {
 	git_merge_file_result result = {0};
-	git_index_entry *ancestor=NULL, ours, *theirs=NULL;
+	git_index_entry *ancestor = NULL, ours, *theirs = NULL;
 
 	git_oid_from_string(&ours.id, "d427e0b2e138501a3d15cc376077a3631e15bd46", GIT_OID_SHA1);
 	ours.path = "automergeable.txt";