[PATCH] send-pack: segfault fix on forced push

[PATCH] send-pack: segfault fix on forced push

[Junio C Hamano]

When pushing to overwrite a ref that points at a commit we do
not even have, the recent "terse push" patch tried to get a
unique abbreviation for the non-existent (from our point of
view) object, which resulted in strcpy(buf, NULL) and
segfaulted.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-send-pack.c         |    5 +++--
 t/t5405-send-pack-rewind.sh |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index c1fd3f5..5a0f5c6 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -365,8 +365,9 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 			char quickref[83];
 			char type = ' ';
 			const char *msg = "";
-
-			strcpy(quickref, find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV));
+			const char *old_abb;
+			old_abb = find_unique_abbrev(ref->old_sha1, DEFAULT_ABBREV);
+			strcpy(quickref, old_abb ? old_abb : old_hex);
 			if (ref_newer(ref->peer_ref->new_sha1, ref->old_sha1))
 				strcat(quickref, "..");
 			else {
diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
new file mode 100755
index 0000000..86abc62
--- /dev/null
+++ b/t/t5405-send-pack-rewind.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+test_description='forced push to replace commit we do not have'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	>file1 && git add file1 && test_tick &&
+	git commit -m Initial &&
+
+	mkdir another && (
+		cd another &&
+		git init &&
+		git fetch .. master:master
+	) &&
+
+	>file2 && git add file2 && test_tick &&
+	git commit -m Second
+
+'
+
+test_expect_success 'non forced push should die not segfault' '
+
+	(
+		cd another &&
+		git push .. master:master
+		test $? = 1
+	)
+
+'
+
+test_expect_success 'forced push should succeed' '
+
+	(
+		cd another &&
+		git push .. +master:master
+	)
+
+'
+
+test_done

Re: [PATCH] send-pack: segfault fix on forced push

[Jeff King]

On Thu, Nov 08, 2007 at 01:38:12AM -0800, Junio C Hamano wrote:

> When pushing to overwrite a ref that points at a commit we do
> not even have, the recent "terse push" patch tried to get a
> unique abbreviation for the non-existent (from our point of
> view) object, which resulted in strcpy(buf, NULL) and
> segfaulted.

Good catch. The fix looks obviously correct.

-Peff