Merge branch 'jk/harden-protocol-v2-delim-handling'

The server-end of the v2 protocol to serve "git clone" and "git
fetch" was not prepared to see a delim packets at unexpected
places, which led to a crash.

* jk/harden-protocol-v2-delim-handling:
  test-lib-functions: simplify packetize() stdin code
  upload-pack: handle unexpected delim packets
  test-lib-functions: make packetize() more efficient

ls-refs.c

@@ -93,7 +93,7 @@ int ls_refs(struct repository *r, struct argv_array *keys,
 
 	git_config(ls_refs_config, NULL);
 
-	while (packet_reader_read(request) != PACKET_READ_FLUSH) {
+	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
 		const char *arg = request->line;
 		const char *out;
 
@@ -105,6 +105,9 @@ int ls_refs(struct repository *r, struct argv_array *keys,
 			argv_array_push(&data.prefixes, out);
 	}
 
+	if (request->status != PACKET_READ_FLUSH)
+		die(_("expected flush after ls-refs arguments"));
+
 	head_ref_namespaced(send_ref, &data);
 	for_each_namespaced_ref(send_ref, &data);
 	packet_flush(1);

t/t5562-http-backend-content-length.sh

@@ -53,15 +53,20 @@ test_expect_success 'setup' '
 	test_commit c1 &&
 	hash_head=$(git rev-parse HEAD) &&
 	hash_prev=$(git rev-parse HEAD~1) &&
-	printf "want %s" "$hash_head" | packetize >fetch_body &&
-	printf 0000 >>fetch_body &&
-	printf "have %s" "$hash_prev" | packetize >>fetch_body &&
-	printf done | packetize >>fetch_body &&
+	{
+		packetize "want $hash_head" &&
+		printf 0000 &&
+		packetize "have $hash_prev" &&
+		packetize "done"
+	} >fetch_body &&
 	test_copy_bytes 10 <fetch_body >fetch_body.trunc &&
 	hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) &&
-	printf "%s %s refs/heads/newbranch\\0report-status\\n" "$ZERO_OID" "$hash_next" | packetize >push_body &&
-	printf 0000 >>push_body &&
-	echo "$hash_next" | git pack-objects --stdout >>push_body &&
+	{
+		printf "%s %s refs/heads/newbranch\\0report-status\\n" \
+			"$ZERO_OID" "$hash_next" | packetize &&
+		printf 0000 &&
+		echo "$hash_next" | git pack-objects --stdout
+	} >push_body &&
 	test_copy_bytes 10 <push_body >push_body.trunc &&
 	: >empty_body
 '

t/t5704-protocol-violations.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='Test responses to violations of the network protocol. In most
+of these cases it will generally be acceptable for one side to break off
+communications if the other side says something unexpected. We are mostly
+making sure that we do not segfault or otherwise behave badly.'
+. ./test-lib.sh
+
+test_expect_success 'extra delim packet in v2 ls-refs args' '
+	{
+		packetize command=ls-refs &&
+		printf 0001 &&
+		# protocol expects 0000 flush here
+		printf 0001
+	} >input &&
+	test_must_fail env GIT_PROTOCOL=version=2 \
+		git upload-pack . <input 2>err &&
+	test_i18ngrep "expected flush after ls-refs arguments" err
+'
+
+test_expect_success 'extra delim packet in v2 fetch args' '
+	{
+		packetize command=fetch &&
+		printf 0001 &&
+		# protocol expects 0000 flush here
+		printf 0001
+	} >input &&
+	test_must_fail env GIT_PROTOCOL=version=2 \
+		git upload-pack . <input 2>err &&
+	test_i18ngrep "expected flush after fetch arguments" err
+'
+
+test_done

t/test-lib-functions.sh

@@ -1362,14 +1362,22 @@ nongit () {
 	)
 } 7>&2 2>&4
 
-# convert stdin to pktline representation; note that empty input becomes an
-# empty packet, not a flush packet (for that you can just print 0000 yourself).
+# convert function arguments or stdin (if not arguments given) to pktline
+# representation. If multiple arguments are given, they are separated by
+# whitespace and put in a single packet. Note that data containing NULs must be
+# given on stdin, and that empty input becomes an empty packet, not a flush
+# packet (for that you can just print 0000 yourself).
 packetize() {
-	cat >packetize.tmp &&
-	len=$(wc -c <packetize.tmp) &&
-	printf '%04x%s' "$(($len + 4))" &&
-	cat packetize.tmp &&
-	rm -f packetize.tmp
+	if test $# -gt 0
+	then
+		packet="$*"
+		printf '%04x%s' "$((4 + ${#packet}))" "$packet"
+	else
+		perl -e '
+			my $packet = do { local $/; <STDIN> };
+			printf "%04x%s", 4 + length($packet), $packet;
+		'
+	fi
 }
 
 # Parse the input as a series of pktlines, writing the result to stdout.

upload-pack.c

@@ -1252,7 +1252,7 @@ static void process_args(struct packet_reader *request,
 			 struct upload_pack_data *data,
 			 struct object_array *want_obj)
 {
-	while (packet_reader_read(request) != PACKET_READ_FLUSH) {
+	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
 		const char *arg = request->line;
 		const char *p;
 
@@ -1321,6 +1321,9 @@ static void process_args(struct packet_reader *request,
 		/* ignore unknown lines maybe? */
 		die("unexpected line: '%s'", arg);
 	}
+
+	if (request->status != PACKET_READ_FLUSH)
+		die(_("expected flush after fetch arguments"));
 }
 
 static int process_haves(struct oid_array *haves, struct oid_array *common,