[PATCH] Segmentation Fault on non-commit --branch clone

[Davide Berardi <berardi.dav@xxxxxxxxx>]

Fixed segmentation fault that can be triggered using
$ git clone --branch $object $repository
with object pointing to a non-commit ref (e.g. a blob).

Signed-off-by: Davide Berardi <berardi.dav@xxxxxxxxx>

---
builtin/clone.c         | 26 ++++++++++++++++++++++++++
refs.h                  |  7 +++++++
t/t5609-clone-branch.sh | 22 +++++++++++++++++++++-
3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f665b28ccc..0f4a18302c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -704,11 +704,32 @@ static void update_remote_refs(const struct ref *refs,
	}
}

+static int fallback_on_noncommit(const struct ref *check,
+				 const struct ref *remote,
+				 const char *msg)
+{
+	if (check == NULL)
+		return 1;
+	struct commit *c = lookup_commit_reference_gently(the_repository,
+						   &check->old_oid, 1);
+	if (c == NULL) {
+		/* Fallback HEAD to fallback refs */
+		warning(_("%s is not a valid commit object, HEAD will fallback to %s"),
+			check->name, FALLBACK_REF);
+		update_ref(msg, FALLBACK_REF, &remote->old_oid, NULL,
+			   REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
+	}
+
+	return c == NULL;
+}
+
static void update_head(const struct ref *our, const struct ref *remote,
			const char *msg)
{
	const char *head;
	if (our && skip_prefix(our->name, "refs/heads/", &head)) {
+		if (fallback_on_noncommit(our, remote, msg) != 0)
+			return;
		/* Local default branch link */
		if (create_symref("HEAD", our->name, NULL) < 0)
			die(_("unable to update HEAD"));
@@ -718,12 +739,17 @@ static void update_head(const struct ref *our, const struct ref *remote,
			install_branch_config(0, head, option_origin, our->name);
		}
	} else if (our) {
+		if (fallback_on_noncommit(our, remote, msg) != 0)
+			return;
+		/* here commit is valid */
		struct commit *c = lookup_commit_reference(the_repository,
							   &our->old_oid);
		/* --branch specifies a non-branch (i.e. tags), detach HEAD */
		update_ref(msg, "HEAD", &c->object.oid, NULL, REF_NO_DEREF,
			   UPDATE_REFS_DIE_ON_ERR);
	} else if (remote) {
+		if (fallback_on_noncommit(remote, remote, msg) != 0)
+			return;
		/*
		 * We know remote HEAD points to a non-branch, or
		 * HEAD points to a branch but we don't know which one.
diff --git a/refs.h b/refs.h
index 730d05ad91..35ee6eb058 100644
--- a/refs.h
+++ b/refs.h
@@ -497,6 +497,13 @@ enum action_on_err {
	UPDATE_REFS_QUIET_ON_ERR
};

+/*
+ * In case of a remote HEAD pointing to a non-commit update_head
+ * will make HEAD reference fallback to this value, master ref
+ * should be safe.
+ */
+#define FALLBACK_REF "refs/heads/master"
+
/*
 * Begin a reference transaction.  The reference transaction must
 * be freed by calling ref_transaction_free().
diff --git a/t/t5609-clone-branch.sh b/t/t5609-clone-branch.sh
index 6e7a7be052..0680cf5a89 100755
--- a/t/t5609-clone-branch.sh
+++ b/t/t5609-clone-branch.sh
@@ -20,7 +20,13 @@ test_expect_success 'setup' '
	 echo one >file && git add file && git commit -m one &&
	 git checkout -b two &&
	 echo two >file && git add file && git commit -m two &&
-	 git checkout master) &&
+	 git checkout master &&
+	 # Create dummy objects
+	 _B=$(git rev-list --objects --all | grep -m 1 "^[^ ]\+ [^ ]\+" | \
+	      awk "{print \$1}") &&
+	 echo "${_B}" >> .git/refs/tags/broken-tag &&
+	 echo "${_B}" >> .git/refs/heads/broken-head
+	) &&
	mkdir empty &&
	(cd empty && git init)
'
@@ -67,4 +73,18 @@ test_expect_success 'clone -b not allowed with empty repos' '
	test_must_fail git clone -b branch empty clone-branch-empty
'

+test_expect_success 'clone -b with broken tag will fallback to master' '
+	git clone -b broken-tag parent clone-broken-tag &&
+	(cd clone-broken-tag &&
+	 check_HEAD master
+	)
+'
+
+test_expect_success 'clone -b with broken head will fallback to master' '
+	git clone -b broken-head parent clone-broken-head &&
+	(cd clone-broken-head &&
+	 check_HEAD master
+	)
+'
+
test_done
--
2.22.0