[PATCH] pull: avoid crash of invalid merge head

[PATCH] pull: avoid crash of invalid merge head

[Jiri Kuncar via GitGitGadget]

From: Jiri Kuncar <jiri.kuncar@gmail.com>

Adds NULL guards for lookup_commit_reference() to avoid segfaults.

Those invalid references are possibly caused by parallel fetches or
gc racing on the same repository.

This effectively treats failed lookup as "not up to date" so caller
falls to a normal merge, which reports the broken object instead of
crashing.

Signed-off-by: Jiri Kuncar <jiri.kuncar@gmail.com>
---
    pull: avoid crash of invalid merge head

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2223%2Fjirikuncar%2Fjk%2Fpull-null-merge-head-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2223/jirikuncar/jk/pull-null-merge-head-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2223

 builtin/pull.c  | 10 +++++++++-
 t/t5520-pull.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index db3ee0aab3..80e79daeb9 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -800,8 +800,12 @@ static int get_can_ff(struct object_id *orig_head,
 
 	orig_merge_head = &merge_heads->oid[0];
 	head = lookup_commit_reference(the_repository, orig_head);
-	commit_list_insert(head, &list);
+	if (!head)
+		return 0;
 	merge_head = lookup_commit_reference(the_repository, orig_merge_head);
+	if (!merge_head)
+		return 0;
+	commit_list_insert(head, &list);
 	ret = repo_is_descendant_of(the_repository, merge_head, list);
 	commit_list_free(list);
 	if (ret < 0)
@@ -820,12 +824,16 @@ static int already_up_to_date(struct object_id *orig_head,
 	struct commit *ours;
 
 	ours = lookup_commit_reference(the_repository, orig_head);
+	if (!ours)
+		return 0;
 	for (size_t i = 0; i < merge_heads->nr; i++) {
 		struct commit_list *list = NULL;
 		struct commit *theirs;
 		int ok;
 
 		theirs = lookup_commit_reference(the_repository, &merge_heads->oid[i]);
+		if (!theirs)
+			return 0;
 		commit_list_insert(theirs, &list);
 		ok = repo_is_descendant_of(the_repository, ours, list);
 		commit_list_free(list);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 27f38ab3c8..7a3eadddd3 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -888,4 +888,30 @@ test_expect_success 'git pull --rebase against local branch' '
 	test_cmp expect file2
 '
 
+test_expect_success 'pull does not crash when a merge head does not resolve' '
+	test_when_finished "rm -rf up dn" &&
+	git init up &&
+	(
+		cd up &&
+		test_commit base &&
+		git switch -c sideA &&
+		test_commit a &&
+		git switch -c sideB base &&
+		test_commit b
+	) &&
+	git clone up dn &&
+	(
+		cd dn &&
+		git -c fetch.unpackLimit=1000 fetch origin \
+			"+refs/heads/*:refs/remotes/origin/*" &&
+		git commit-graph write --reachable &&
+		oid=$(git rev-parse refs/remotes/origin/sideA) &&
+		obj=.git/objects/$(test_oid_to_path "$oid") &&
+		test -f "$obj" &&
+		chmod u+w "$obj" &&
+		>"$obj" &&
+		test_must_fail git pull --no-rebase origin sideA sideB
+	)
+'
+
 test_done

base-commit: fa7f9290efe2bd22dd736689597b474b93798e11
-- 
gitgitgadget