From da70156ef3c5a2903e7a337a9e130151d0db8a3a Mon Sep 17 00:00:00 2001
From: Nelson Elhage <nelhage@nelhage.com>
Date: Sat, 25 Aug 2018 17:04:39 +0000
Subject: [PATCH 01/28] config: convert unbounded recursion into a loop

(cherry picked from commit a03113e80332fba6c77f43b21d398caad50b4b89)
---
 src/config_file.c | 62 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 33 deletions(-)

diff --git a/src/config_file.c b/src/config_file.c
index e15d57bbba5..ce1743720f3 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1349,46 +1349,42 @@ static int parse_multiline_variable(struct reader *reader, git_buf *value, int i
 {
 	char *line = NULL, *proc_line = NULL;
 	int quote_count;
-	bool multiline;
+	bool multiline = true;
+
+	while (multiline) {
+		/* Check that the next line exists */
+		line = reader_readline(reader, false);
+		if (line == NULL)
+			return -1;
+
+		/* We've reached the end of the file, there is no continuation.
+		 * (this is not an error).
+		 */
+		if (line[0] == '\0') {
+			git__free(line);
+			return 0;
+		}
 
-	/* Check that the next line exists */
-	line = reader_readline(reader, false);
-	if (line == NULL)
-		return -1;
+		quote_count = strip_comments(line, !!in_quotes);
 
-	/* We've reached the end of the file, there is no continuation.
-	 * (this is not an error).
-	 */
-	if (line[0] == '\0') {
-		git__free(line);
-		return 0;
-	}
+		/* If it was just a comment, pretend it didn't exist */
+		if (line[0] == '\0') {
+			in_quotes = quote_count;
+			continue;
+		}
 
-	quote_count = strip_comments(line, !!in_quotes);
+		if (unescape_line(&proc_line, &multiline, line, in_quotes) < 0) {
+			git__free(line);
+			return -1;
+		}
+		/* add this line to the multiline var */
 
-	/* If it was just a comment, pretend it didn't exist */
-	if (line[0] == '\0') {
+		git_buf_puts(value, proc_line);
 		git__free(line);
-		return parse_multiline_variable(reader, value, quote_count);
-		/* TODO: unbounded recursion. This **could** be exploitable */
-	}
+		git__free(proc_line);
 
-	if (unescape_line(&proc_line, &multiline, line, in_quotes) < 0) {
-		git__free(line);
-		return -1;
+		in_quotes = quote_count;
 	}
-	/* add this line to the multiline var */
-
-	git_buf_puts(value, proc_line);
-	git__free(line);
-	git__free(proc_line);
-
-	/*
-	 * If we need to continue reading the next line, let's just
-	 * keep putting stuff in the buffer
-	 */
-	if (multiline)
-		return parse_multiline_variable(reader, value, quote_count);
 
 	return 0;
 }

From c18c913c3d57bc0ab5dc3c0abb14de263e9a4f76 Mon Sep 17 00:00:00 2001
From: Nelson Elhage <nelhage@nelhage.com>
Date: Sat, 1 Sep 2018 03:50:26 +0000
Subject: [PATCH 02/28] config: Fix a leak parsing multi-line config entries

(cherry picked from commit 38b852558eb518f96c313cdcd9ce5a7af6ded194)
---
 src/config_file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/config_file.c b/src/config_file.c
index ce1743720f3..840e9aaccdb 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1369,6 +1369,7 @@ static int parse_multiline_variable(struct reader *reader, git_buf *value, int i
 
 		/* If it was just a comment, pretend it didn't exist */
 		if (line[0] == '\0') {
+			git__free(line);
 			in_quotes = quote_count;
 			continue;
 		}

From c4db1715478bd0284fcc6d730c4147c252b2615f Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 3 Sep 2018 10:49:46 +0200
Subject: [PATCH 03/28] config_parse: refactor error handling when parsing
 multiline variables

The current error handling for the multiline variable parser is a bit
fragile, as each error condition has its own code to clear memory.
Instead, unify error handling as far as possible to avoid this
repetitive code. While at it, make use of `GITERR_CHECK_ALLOC` to
correctly handle OOM situations and verify that the buffer we print into
does not run out of memory either.

(cherry picked from commit bc63e1ef521ab5900dc0b0dcd578b8bf18627fb1)
---
 src/config_file.c | 43 ++++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/config_file.c b/src/config_file.c
index 840e9aaccdb..81339b455b2 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1347,44 +1347,49 @@ static int unescape_line(
 
 static int parse_multiline_variable(struct reader *reader, git_buf *value, int in_quotes)
 {
-	char *line = NULL, *proc_line = NULL;
 	int quote_count;
 	bool multiline = true;
 
 	while (multiline) {
+		char *line = NULL, *proc_line = NULL;
+		int error;
+
 		/* Check that the next line exists */
 		line = reader_readline(reader, false);
-		if (line == NULL)
-			return -1;
+		GITERR_CHECK_ALLOC(line);
 
-		/* We've reached the end of the file, there is no continuation.
+		/*
+		 * We've reached the end of the file, there is no continuation.
 		 * (this is not an error).
 		 */
 		if (line[0] == '\0') {
-			git__free(line);
-			return 0;
+			error = 0;
+			goto out;
 		}
 
+		/* If it was just a comment, pretend it didn't exist */
 		quote_count = strip_comments(line, !!in_quotes);
+		if (line[0] == '\0')
+			goto next;
 
-		/* If it was just a comment, pretend it didn't exist */
-		if (line[0] == '\0') {
-			git__free(line);
-			in_quotes = quote_count;
-			continue;
-		}
+		if ((error = unescape_line(&proc_line, &multiline,
+					   line, in_quotes)) < 0)
+			goto out;
 
-		if (unescape_line(&proc_line, &multiline, line, in_quotes) < 0) {
-			git__free(line);
-			return -1;
-		}
-		/* add this line to the multiline var */
+		/* Add this line to the multiline var */
+		if ((error = git_buf_puts(value, proc_line)) < 0)
+			goto out;
 
-		git_buf_puts(value, proc_line);
+next:
 		git__free(line);
 		git__free(proc_line);
-
 		in_quotes = quote_count;
+		continue;
+
+out:
+		git__free(line);
+		git__free(proc_line);
+		return error;
 	}
 
 	return 0;

From ffc205644e76f99d1802fe4229cb7c209f3646c4 Mon Sep 17 00:00:00 2001
From: bisho <bishillo@gmail.com>
Date: Wed, 5 Sep 2018 11:49:13 -0700
Subject: [PATCH 04/28] Prevent heap-buffer-overflow

When running repack while doing repo writes, `packfile_load__cb()` can see some temporary files in the directory that are bigger than the usual, and makes `memcmp` overflow on the `p->pack_name` string. ASAN detected this. This just uses `strncmp`, that should not have any performance impact and is safe for comparing strings of different sizes.

```
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61200001a3f3 at pc 0x7f4a9e1976ec bp 0x7ffc1f80e100 sp 0x7ffc1f80d8b0
READ of size 89 at 0x61200001a3f3 thread T0
SCARINESS: 26 (multi-byte-read-heap-buffer-overflow)
    #0 0x7f4a9e1976eb in __interceptor_memcmp.part.78 (/build/cfgr-admin#link-tree/libtools_build_sanitizers_asan-ubsan-py.so+0xcf6eb)
    #1 0x7f4a518c5431 in packfile_load__cb /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:213
    #2 0x7f4a518d9582 in git_path_direach /build/libgit2/0.27.0/src/libgit2-0.27.0/src/path.c:1134
    #3 0x7f4a518c58ad in pack_backend__refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb_pack.c:347
    #4 0x7f4a518c1b12 in git_odb_refresh /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1511
    #5 0x7f4a518bff5f in git_odb__freshen /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:752
    #6 0x7f4a518c17d4 in git_odb_stream_finalize_write /build/libgit2/0.27.0/src/libgit2-0.27.0/src/odb.c:1415
    #7 0x7f4a51b9d015 in Repository_write /build/pygit2/0.27.0/src/pygit2-0.27.0/src/repository.c:509
```

(cherry picked from commit d22cd1f4a4c10ff47b04c57560e6765d77e5a8fd)
---
 src/odb_pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/odb_pack.c b/src/odb_pack.c
index 51770a88e21..d5120797541 100644
--- a/src/odb_pack.c
+++ b/src/odb_pack.c
@@ -209,7 +209,7 @@ static int packfile_load__cb(void *data, git_buf *path)
 	for (i = 0; i < backend->packs.length; ++i) {
 		struct git_pack_file *p = git_vector_get(&backend->packs, i);
 
-		if (memcmp(p->pack_name, path_str, cmp_len) == 0)
+		if (strncmp(p->pack_name, path_str, cmp_len) == 0)
 			return 0;
 	}
 

From 8ced462701e5e0986b989faa8af672756a6091e0 Mon Sep 17 00:00:00 2001
From: Christian Schlack <christian@backhub.co>
Date: Sat, 11 Aug 2018 13:06:14 +0200
Subject: [PATCH 05/28] Fix 'invalid packet line' for ng packets containing
 errors

(cherry picked from commit 50dd7fea5ad1bf6c013b72ad0aa803a9c84cdede)
---
 src/transports/smart_pkt.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index d10d6c68fae..d66ee3e929b 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -290,7 +290,7 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len)
 static int ng_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_ng *pkt;
-	const char *ptr;
+	const char *ptr, *eol;
 	size_t alloclen;
 
 	pkt = git__malloc(sizeof(*pkt));
@@ -299,11 +299,13 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len)
 	pkt->ref = NULL;
 	pkt->type = GIT_PKT_NG;
 
+	eol = line + len;
+
 	if (len < 3)
 		goto out_err;
 	line += 3; /* skip "ng " */
-	len -= 3;
-	if (!(ptr = memchr(line, ' ', len)))
+
+	if (!(ptr = memchr(line, ' ', eol - line)))
 		goto out_err;
 	len = ptr - line;
 
@@ -314,11 +316,11 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len)
 	memcpy(pkt->ref, line, len);
 	pkt->ref[len] = '\0';
 
-	if (len < 1)
-		goto out_err;
 	line = ptr + 1;
-	len -= 1;
-	if (!(ptr = memchr(line, '\n', len)))
+	if (line >= eol)
+		goto out_err;
+
+	if (!(ptr = memchr(line, '\n', eol - line)))
 		goto out_err;
 	len = ptr - line;
 

From c83c59b82bc5aa26b5387714f26bdabf8e15c3e1 Mon Sep 17 00:00:00 2001
From: Nelson Elhage <nelhage@nelhage.com>
Date: Tue, 26 Jun 2018 02:32:50 +0000
Subject: [PATCH 06/28] Remove GIT_PKT_PACK entirely

(cherry picked from commit 90cf86070046fcffd5306915b57786da054d8964)
---
 src/transports/smart.h     |  2 +-
 src/transports/smart_pkt.c | 27 ++-------------------------
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/src/transports/smart.h b/src/transports/smart.h
index b47001fe004..b5ca46d34a3 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -35,7 +35,7 @@ enum git_pkt_type {
 	GIT_PKT_HAVE,
 	GIT_PKT_ACK,
 	GIT_PKT_NAK,
-	GIT_PKT_PACK,
+	GIT_PKT_PACK__UNUSED,
 	GIT_PKT_COMMENT,
 	GIT_PKT_ERR,
 	GIT_PKT_DATA,
diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index d66ee3e929b..23772445ca4 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -86,19 +86,6 @@ static int nak_pkt(git_pkt **out)
 	return 0;
 }
 
-static int pack_pkt(git_pkt **out)
-{
-	git_pkt *pkt;
-
-	pkt = git__malloc(sizeof(git_pkt));
-	GITERR_CHECK_ALLOC(pkt);
-
-	pkt->type = GIT_PKT_PACK;
-	*out = pkt;
-
-	return 0;
-}
-
 static int comment_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_comment *pkt;
@@ -378,7 +365,7 @@ static int32_t parse_len(const char *line)
 					num[k] = '.';
 				}
 			}
-			
+
 			giterr_set(GITERR_NET, "invalid hex digit in length: '%s'", num);
 			return -1;
 		}
@@ -415,17 +402,7 @@ int git_pkt_parse_line(
 
 	len = parse_len(line);
 	if (len < 0) {
-		/*
-		 * If we fail to parse the length, it might be because the
-		 * server is trying to send us the packfile already.
-		 */
-		if (bufflen >= 4 && !git__prefixcmp(line, "PACK")) {
-			giterr_clear();
-			*out = line;
-			return pack_pkt(head);
-		}
-
-		return (int)len;
+		return GIT_ERROR;
 	}
 
 	/*

From e91024e1adea7f272d8270c0787aeaa3857a2855 Mon Sep 17 00:00:00 2001
From: Nelson Elhage <nelhage@nelhage.com>
Date: Thu, 28 Jun 2018 05:27:36 +0000
Subject: [PATCH 07/28] Small style tweak, and set an error

(cherry picked from commit 895a668e19dc596e7b12ea27724ceb7b68556106)
---
 src/transports/smart_pkt.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 23772445ca4..d4fbd3779ea 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -402,7 +402,17 @@ int git_pkt_parse_line(
 
 	len = parse_len(line);
 	if (len < 0) {
-		return GIT_ERROR;
+		/*
+		 * If we fail to parse the length, it might be because the
+		 * server is trying to send us the packfile already.
+		 */
+		if (bufflen >= 4 && !git__prefixcmp(line, "PACK")) {
+			giterr_set(GITERR_NET, "unexpected pack file");
+		} else {
+			giterr_set(GITERR_NET, "bad packet length");
+		}
+
+		return -1;
 	}
 
 	/*

From 9561ec83c09ffd18bd9967647eac70a7df95ece0 Mon Sep 17 00:00:00 2001
From: Etienne Samson <samson.etienne@gmail.com>
Date: Tue, 22 Aug 2017 16:29:07 +0200
Subject: [PATCH 08/28] smart: typedef git_pkt_type and clarify recv_pkt return
 type

(cherry picked from commit 08961c9d0d6927bfcc725bd64c9a87dbcca0c52c)
---
 src/transports/smart.h          | 24 +++++++++++-----------
 src/transports/smart_protocol.c | 35 ++++++++++++++++-----------------
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/src/transports/smart.h b/src/transports/smart.h
index b5ca46d34a3..b9f1c6fb9c9 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -28,7 +28,7 @@
 
 extern bool git_smart__ofs_delta_enabled;
 
-enum git_pkt_type {
+typedef enum {
 	GIT_PKT_CMD,
 	GIT_PKT_FLUSH,
 	GIT_PKT_REF,
@@ -43,7 +43,7 @@ enum git_pkt_type {
 	GIT_PKT_OK,
 	GIT_PKT_NG,
 	GIT_PKT_UNPACK,
-};
+} git_pkt_type;
 
 /* Used for multi_ack and mutli_ack_detailed */
 enum git_ack_status {
@@ -55,11 +55,11 @@ enum git_ack_status {
 
 /* This would be a flush pkt */
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 } git_pkt;
 
 struct git_pkt_cmd {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	char *cmd;
 	char *path;
 	char *host;
@@ -67,25 +67,25 @@ struct git_pkt_cmd {
 
 /* This is a pkt-line with some info in it */
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	git_remote_head head;
 	char *capabilities;
 } git_pkt_ref;
 
 /* Useful later */
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	git_oid oid;
 	enum git_ack_status status;
 } git_pkt_ack;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	char comment[GIT_FLEX_ARRAY];
 } git_pkt_comment;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	int len;
 	char data[GIT_FLEX_ARRAY];
 } git_pkt_data;
@@ -93,24 +93,24 @@ typedef struct {
 typedef git_pkt_data git_pkt_progress;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	int len;
 	char error[GIT_FLEX_ARRAY];
 } git_pkt_err;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	char *ref;
 } git_pkt_ok;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	char *ref;
 	char *msg;
 } git_pkt_ng;
 
 typedef struct {
-	enum git_pkt_type type;
+	git_pkt_type type;
 	int unpack_ok;
 } git_pkt_unpack;
 
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index eab10aac69e..892287a7c47 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -206,11 +206,11 @@ int git_smart__detect_caps(git_pkt_ref *pkt, transport_smart_caps *caps, git_vec
 	return 0;
 }
 
-static int recv_pkt(git_pkt **out, gitno_buffer *buf)
+static int recv_pkt(git_pkt **out, git_pkt_type *pkt_type, gitno_buffer *buf)
 {
 	const char *ptr = buf->data, *line_end = ptr;
 	git_pkt *pkt = NULL;
-	int pkt_type, error = 0, ret;
+	int error = 0, ret;
 
 	do {
 		if (buf->offset > 0)
@@ -233,13 +233,14 @@ static int recv_pkt(git_pkt **out, gitno_buffer *buf)
 	} while (error);
 
 	gitno_consume(buf, line_end);
-	pkt_type = pkt->type;
+	if (pkt_type)
+		*pkt_type = pkt->type;
 	if (out != NULL)
 		*out = pkt;
 	else
 		git__free(pkt);
 
-	return pkt_type;
+	return error;
 }
 
 static int store_common(transport_smart *t)
@@ -249,7 +250,7 @@ static int store_common(transport_smart *t)
 	int error;
 
 	do {
-		if ((error = recv_pkt(&pkt, buf)) < 0)
+		if ((error = recv_pkt(&pkt, NULL, buf)) < 0)
 			return error;
 
 		if (pkt->type == GIT_PKT_ACK) {
@@ -317,7 +318,7 @@ static int wait_while_ack(gitno_buffer *buf)
 	while (1) {
 		git__free(pkt);
 
-		if ((error = recv_pkt((git_pkt **)&pkt, buf)) < 0)
+		if ((error = recv_pkt((git_pkt **)&pkt, NULL, buf)) < 0)
 			return error;
 
 		if (pkt->type == GIT_PKT_NAK)
@@ -342,7 +343,8 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 	gitno_buffer *buf = &t->buffer;
 	git_buf data = GIT_BUF_INIT;
 	git_revwalk *walk = NULL;
-	int error = -1, pkt_type;
+	int error = -1;
+	git_pkt_type pkt_type;
 	unsigned int i;
 	git_oid oid;
 
@@ -392,16 +394,13 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 				if ((error = store_common(t)) < 0)
 					goto on_error;
 			} else {
-				pkt_type = recv_pkt(NULL, buf);
-
-				if (pkt_type == GIT_PKT_ACK) {
+				error = recv_pkt(NULL, &pkt_type, buf);
+				if (error < 0) {
+					goto on_error;
+				} else if (pkt_type == GIT_PKT_ACK) {
 					break;
 				} else if (pkt_type == GIT_PKT_NAK) {
 					continue;
-				} else if (pkt_type < 0) {
-					/* recv_pkt returned an error */
-					error = pkt_type;
-					goto on_error;
 				} else {
 					giterr_set(GITERR_NET, "Unexpected pkt type");
 					error = -1;
@@ -467,10 +466,10 @@ int git_smart__negotiate_fetch(git_transport *transport, git_repository *repo, c
 
 	/* Now let's eat up whatever the server gives us */
 	if (!t->caps.multi_ack && !t->caps.multi_ack_detailed) {
-		pkt_type = recv_pkt(NULL, buf);
+		error = recv_pkt(NULL, &pkt_type, buf);
 
-		if (pkt_type < 0) {
-			return pkt_type;
+		if (error < 0) {
+			return error;
 		} else if (pkt_type != GIT_PKT_ACK && pkt_type != GIT_PKT_NAK) {
 			giterr_set(GITERR_NET, "Unexpected pkt type");
 			return -1;
@@ -591,7 +590,7 @@ int git_smart__download_pack(
 			goto done;
 		}
 
-		if ((error = recv_pkt(&pkt, buf)) >= 0) {
+		if ((error = recv_pkt(&pkt, NULL, buf)) >= 0) {
 			/* Check cancellation after network call */
 			if (t->cancelled.val) {
 				giterr_clear();

From 5f5577809fcde9dfb974bcbf4cf0bc70b485e287 Mon Sep 17 00:00:00 2001
From: Nelson Elhage <nelhage@nelhage.com>
Date: Sun, 24 Jun 2018 19:47:08 +0000
Subject: [PATCH 09/28] Verify ref_pkt's are long enough

If the remote sends a too-short packet, we'll allow `len` to go
negative and eventually issue a malloc for <= 0 bytes on

```
pkt->head.name = git__malloc(alloclen);
```

(cherry picked from commit 437ee5a70711ac2e027877d71ee4ae17e5ec3d6c)
---
 src/transports/smart_pkt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index d4fbd3779ea..9b7e3bc4ed6 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -203,6 +203,11 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len)
 	git_pkt_ref *pkt;
 	size_t alloclen;
 
+	if (len < GIT_OID_HEXSZ + 1) {
+		giterr_set(GITERR_NET, "error parsing pkt-line");
+		return -1;
+	}
+
 	pkt = git__malloc(sizeof(git_pkt_ref));
 	GITERR_CHECK_ALLOC(pkt);
 

From a8db6c92ae8ea0e219ccce40d9be452d9a0de679 Mon Sep 17 00:00:00 2001
From: Edward Thomson <ethomson@edwardthomson.com>
Date: Thu, 30 Nov 2017 15:40:13 +0000
Subject: [PATCH 10/28] util: introduce `git__prefixncmp` and consolidate
 implementations

Introduce `git_prefixncmp` that will search up to the first `n`
characters of a string to see if it is prefixed by another string.
This is useful for examining if a non-null terminated character
array is prefixed by a particular substring.

Consolidate the various implementations of `git__prefixcmp` around a
single core implementation and add some test cases to validate its
behavior.

(cherry picked from commit 86219f40689c85ec4418575223f4376beffa45af)
---
 src/util.c          | 44 ++++++++++++++++++++++++++++----------------
 src/util.h          |  1 +
 tests/core/string.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 16 deletions(-)

diff --git a/src/util.c b/src/util.c
index a44f4c9acfc..964b0ab6a64 100644
--- a/src/util.c
+++ b/src/util.c
@@ -250,35 +250,47 @@ void git__strtolower(char *str)
 	git__strntolower(str, strlen(str));
 }
 
-int git__prefixcmp(const char *str, const char *prefix)
+GIT_INLINE(int) prefixcmp(const char *str, size_t str_n, const char *prefix, bool icase)
 {
-	for (;;) {
-		unsigned char p = *(prefix++), s;
+	int s, p;
+
+	while (str_n--) {
+		s = (unsigned char)*str++;
+		p = (unsigned char)*prefix++;
+
+		if (icase) {
+			s = git__tolower(s);
+			p = git__tolower(p);
+		}
+
 		if (!p)
 			return 0;
-		if ((s = *(str++)) != p)
+
+		if (s != p)
 			return s - p;
 	}
+
+	return (0 - *prefix);
 }
 
-int git__prefixcmp_icase(const char *str, const char *prefix)
+int git__prefixcmp(const char *str, const char *prefix)
 {
-	return strncasecmp(str, prefix, strlen(prefix));
+	return prefixcmp(str, SIZE_MAX, prefix, false);
 }
 
-int git__prefixncmp_icase(const char *str, size_t str_n, const char *prefix)
+int git__prefixncmp(const char *str, size_t str_n, const char *prefix)
 {
-	int s, p;
-
-	while(str_n--) {
-		s = (unsigned char)git__tolower(*str++);
-		p = (unsigned char)git__tolower(*prefix++);
+	return prefixcmp(str, str_n, prefix, false);
+}
 
-		if (s != p)
-			return s - p;
-	}
+int git__prefixcmp_icase(const char *str, const char *prefix)
+{
+	return prefixcmp(str, SIZE_MAX, prefix, true);
+}
 
-	return (0 - *prefix);
+int git__prefixncmp_icase(const char *str, size_t str_n, const char *prefix)
+{
+	return prefixcmp(str, str_n, prefix, true);
 }
 
 int git__suffixcmp(const char *str, const char *suffix)
diff --git a/src/util.h b/src/util.h
index eb15250d8dd..8e666f9de2d 100644
--- a/src/util.h
+++ b/src/util.h
@@ -254,6 +254,7 @@ GIT_INLINE(void) git__free(void *ptr)
 
 extern int git__prefixcmp(const char *str, const char *prefix);
 extern int git__prefixcmp_icase(const char *str, const char *prefix);
+extern int git__prefixncmp(const char *str, size_t str_n, const char *prefix);
 extern int git__prefixncmp_icase(const char *str, size_t str_n, const char *prefix);
 extern int git__suffixcmp(const char *str, const char *suffix);
 
diff --git a/tests/core/string.c b/tests/core/string.c
index 90e8fa027cf..85db0c66230 100644
--- a/tests/core/string.c
+++ b/tests/core/string.c
@@ -40,6 +40,48 @@ void test_core_string__2(void)
 	cl_assert(git__strcasesort_cmp("fooBar", "foobar") < 0);
 }
 
+/* compare prefixes with len */
+void test_core_string__prefixncmp(void)
+{
+	cl_assert(git__prefixncmp("", 0, "") == 0);
+	cl_assert(git__prefixncmp("a", 1, "") == 0);
+	cl_assert(git__prefixncmp("", 0, "a") < 0);
+	cl_assert(git__prefixncmp("a", 1, "b") < 0);
+	cl_assert(git__prefixncmp("b", 1, "a") > 0);
+	cl_assert(git__prefixncmp("ab", 2, "a") == 0);
+	cl_assert(git__prefixncmp("ab", 1, "a") == 0);
+	cl_assert(git__prefixncmp("ab", 2, "ac") < 0);
+	cl_assert(git__prefixncmp("a", 1, "ac") < 0);
+	cl_assert(git__prefixncmp("ab", 1, "ac") < 0);
+	cl_assert(git__prefixncmp("ab", 2, "aa") > 0);
+	cl_assert(git__prefixncmp("ab", 1, "aa") < 0);
+}
+
+/* compare prefixes with len */
+void test_core_string__prefixncmp_icase(void)
+{
+	cl_assert(git__prefixncmp_icase("", 0, "") == 0);
+	cl_assert(git__prefixncmp_icase("a", 1, "") == 0);
+	cl_assert(git__prefixncmp_icase("", 0, "a") < 0);
+	cl_assert(git__prefixncmp_icase("a", 1, "b") < 0);
+	cl_assert(git__prefixncmp_icase("A", 1, "b") < 0);
+	cl_assert(git__prefixncmp_icase("a", 1, "B") < 0);
+	cl_assert(git__prefixncmp_icase("b", 1, "a") > 0);
+	cl_assert(git__prefixncmp_icase("B", 1, "a") > 0);
+	cl_assert(git__prefixncmp_icase("b", 1, "A") > 0);
+	cl_assert(git__prefixncmp_icase("ab", 2, "a") == 0);
+	cl_assert(git__prefixncmp_icase("Ab", 2, "a") == 0);
+	cl_assert(git__prefixncmp_icase("ab", 2, "A") == 0);
+	cl_assert(git__prefixncmp_icase("ab", 1, "a") == 0);
+	cl_assert(git__prefixncmp_icase("ab", 2, "ac") < 0);
+	cl_assert(git__prefixncmp_icase("Ab", 2, "ac") < 0);
+	cl_assert(git__prefixncmp_icase("ab", 2, "Ac") < 0);
+	cl_assert(git__prefixncmp_icase("a", 1, "ac") < 0);
+	cl_assert(git__prefixncmp_icase("ab", 1, "ac") < 0);
+	cl_assert(git__prefixncmp_icase("ab", 2, "aa") > 0);
+	cl_assert(git__prefixncmp_icase("ab", 1, "aa") < 0);
+}
+
 void test_core_string__strcmp(void)
 {
 	cl_assert(git__strcmp("", "") == 0);

From 5d108c9ab865a6b616a3daca173f8d1eb069fcd0 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 3 Oct 2018 15:39:40 +0200
Subject: [PATCH 11/28] tests: verify parsing logic for smart packets

The commits following this commit are about to introduce quite a lot of
refactoring and tightening of the smart packet parser. Unfortunately, we
do not yet have any tests despite our online tests that verify that our
parser does not regress upon changes. This is doubly unfortunate as our
online tests aren't executed by default.

Add new tests that exercise the smart parsing logic directly by
executing `git_pkt_parse_line`.

(cherry picked from commit 365d2720c1a5fc89f03fd85265c8b45195c7e4a8)
---
 tests/transports/smart/packet.c | 348 ++++++++++++++++++++++++++++++++
 1 file changed, 348 insertions(+)
 create mode 100644 tests/transports/smart/packet.c

diff --git a/tests/transports/smart/packet.c b/tests/transports/smart/packet.c
new file mode 100644
index 00000000000..2b9b38f5c77
--- /dev/null
+++ b/tests/transports/smart/packet.c
@@ -0,0 +1,348 @@
+#include "clar_libgit2.h"
+#include "transports/smart.h"
+
+enum expected_status {
+	PARSE_SUCCESS,
+	PARSE_FAILURE
+};
+
+static void assert_flush_parses(const char *line)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_FLUSH);
+	cl_assert_equal_strn(endptr, line + 4, linelen - 4);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_data_pkt_parses(const char *line, const char *expected_data, size_t expected_len)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_data *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_DATA);
+	cl_assert_equal_i(pkt->len, expected_len);
+	cl_assert_equal_strn(pkt->data, expected_data, expected_len);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_sideband_progress_parses(const char *line, const char *expected_data, size_t expected_len)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_progress *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_PROGRESS);
+	cl_assert_equal_i(pkt->len, expected_len);
+	cl_assert_equal_strn(pkt->data, expected_data, expected_len);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_error_parses(const char *line, const char *expected_error, size_t expected_len)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_err *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_ERR);
+	cl_assert_equal_i(pkt->len, expected_len);
+	cl_assert_equal_strn(pkt->error, expected_error, expected_len);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_ack_parses(const char *line, const char *expected_oid, enum git_ack_status expected_status)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_ack *pkt;
+	git_oid oid;
+
+	cl_git_pass(git_oid_fromstr(&oid, expected_oid));
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_ACK);
+	cl_assert_equal_oid(&pkt->oid, &oid);
+	cl_assert_equal_i(pkt->status, expected_status);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_nak_parses(const char *line)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_NAK);
+	cl_assert_equal_strn(endptr, line + 7, linelen - 7);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_comment_parses(const char *line, const char *expected_comment)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_comment *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_COMMENT);
+	cl_assert_equal_strn(pkt->comment, expected_comment, strlen(expected_comment));
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_ok_parses(const char *line, const char *expected_ref)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_ok *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_OK);
+	cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_unpack_parses(const char *line, bool ok)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_unpack *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_UNPACK);
+	cl_assert_equal_i(pkt->unpack_ok, ok);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_ng_parses(const char *line, const char *expected_ref, const char *expected_msg)
+{
+	size_t linelen = strlen(line) + 1;
+	const char *endptr;
+	git_pkt_ng *pkt;
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_NG);
+	cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));
+	cl_assert_equal_strn(pkt->msg, expected_msg, strlen(expected_msg));
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+#define assert_ref_parses(line, expected_oid, expected_ref, expected_capabilities) \
+	assert_ref_parses_(line, sizeof(line), expected_oid, expected_ref, expected_capabilities)
+
+static void assert_ref_parses_(const char *line, size_t linelen, const char *expected_oid,
+	const char *expected_ref, const char *expected_capabilities)
+{
+	const char *endptr;
+	git_pkt_ref *pkt;
+	git_oid oid;
+
+	cl_git_pass(git_oid_fromstr(&oid, expected_oid));
+
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_assert_equal_i(pkt->type, GIT_PKT_REF);
+	cl_assert_equal_oid(&pkt->head.oid, &oid);
+	cl_assert_equal_strn(pkt->head.name, expected_ref, strlen(expected_ref));
+	if (expected_capabilities)
+		cl_assert_equal_strn(pkt->capabilities, expected_capabilities, strlen(expected_capabilities));
+	else
+		cl_assert_equal_p(NULL, pkt->capabilities);
+
+	git_pkt_free((git_pkt *) pkt);
+}
+
+static void assert_pkt_fails(const char *line)
+{
+	const char *endptr;
+	git_pkt *pkt;
+	cl_git_fail(git_pkt_parse_line(&pkt, line, &endptr, strlen(line) + 1));
+}
+
+void test_transports_smart_packet__parsing_garbage_fails(void)
+{
+	assert_pkt_fails("0foobar");
+	assert_pkt_fails("00foobar");
+	assert_pkt_fails("000foobar");
+	assert_pkt_fails("0001");
+	assert_pkt_fails("");
+	assert_pkt_fails("0");
+	assert_pkt_fails("0i00");
+	assert_pkt_fails("f");
+}
+
+void test_transports_smart_packet__flush_parses(void)
+{
+	assert_flush_parses("0000");
+	assert_flush_parses("0000foobar");
+}
+
+void test_transports_smart_packet__data_pkt(void)
+{
+	assert_pkt_fails("000foobar");
+	assert_pkt_fails("0001o");
+	assert_pkt_fails("0001\1");
+	assert_data_pkt_parses("0005\1", "", 0);
+	assert_pkt_fails("0009\1o");
+	assert_data_pkt_parses("0009\1data", "data", 4);
+	assert_data_pkt_parses("000a\1data", "data", 5);
+}
+
+void test_transports_smart_packet__sideband_progress_pkt(void)
+{
+	assert_pkt_fails("0001\2");
+	assert_sideband_progress_parses("0005\2", "", 0);
+	assert_pkt_fails("0009\2o");
+	assert_sideband_progress_parses("0009\2data", "data", 4);
+	assert_sideband_progress_parses("000a\2data", "data", 5);
+}
+
+void test_transports_smart_packet__sideband_err_pkt(void)
+{
+	assert_pkt_fails("0001\3");
+	assert_error_parses("0005\3", "", 0);
+	assert_pkt_fails("0009\3o");
+	assert_error_parses("0009\3data", "data", 4);
+	assert_error_parses("000a\3data", "data", 5);
+}
+
+void test_transports_smart_packet__ack_pkt(void)
+{
+	assert_ack_parses("0030ACK 0000000000000000000000000000000000000000",
+			  "0000000000000000000000000000000000000000", 0);
+	assert_ack_parses("0039ACK 0000000000000000000000000000000000000000 continue",
+			  "0000000000000000000000000000000000000000",
+			  GIT_ACK_CONTINUE);
+	assert_ack_parses("0037ACK 0000000000000000000000000000000000000000 common",
+			  "0000000000000000000000000000000000000000",
+			  GIT_ACK_COMMON);
+	assert_ack_parses("0037ACK 0000000000000000000000000000000000000000 ready",
+			  "0000000000000000000000000000000000000000",
+			  GIT_ACK_READY);
+
+	/* TODO: these should fail as they don't have OIDs */
+	assert_ack_parses("0007ACK", "0000000000000000000000000000000000000000", 0);
+	assert_ack_parses("0008ACK ", "0000000000000000000000000000000000000000", 0);
+
+	/* TODO: this one is missing a space and should thus fail */
+	assert_ack_parses("0036ACK00000000000000000x0000000000000000000000 ready",
+			  "0000000000000000000000000000000000000000", 0);
+
+	/* TODO: the following ones have invalid OIDs and should thus fail */
+	assert_ack_parses("0037ACK 00000000000000000x0000000000000000000000 ready",
+			  "0000000000000000000000000000000000000000", GIT_ACK_READY);
+	assert_ack_parses("0036ACK 000000000000000000000000000000000000000 ready",
+			  "0000000000000000000000000000000000000000", 0);
+	assert_ack_parses("0036ACK 00000000000000000x0000000000000000000000ready",
+			  "0000000000000000000000000000000000000000", 0);
+
+	/* TODO: this one has an invalid status and should thus fail */
+	assert_ack_parses("0036ACK 0000000000000000000000000000000000000000 read",
+			  "0000000000000000000000000000000000000000", 0);
+}
+
+void test_transports_smart_packet__nak_pkt(void)
+{
+	assert_nak_parses("0007NAK");
+	assert_pkt_fails("0007NaK");
+	assert_pkt_fails("0007nak");
+	assert_nak_parses("0007NAKfoobar");
+	assert_pkt_fails("0007nakfoobar");
+	assert_pkt_fails("0007 NAK");
+}
+
+void test_transports_smart_packet__error_pkt(void)
+{
+	assert_pkt_fails("0007ERR");
+	assert_pkt_fails("0008ERRx");
+	assert_error_parses("0008ERR ", "", 0);
+	assert_error_parses("000EERR ERRMSG", "ERRMSG", 6);
+}
+
+void test_transports_smart_packet__comment_pkt(void)
+{
+	assert_comment_parses("0005#", "");
+	assert_comment_parses("000B#foobar", "#fooba");
+	assert_comment_parses("000C#foobar", "#foobar");
+	assert_comment_parses("001A#this is a comment\nfoo", "#this is a comment\nfoo");
+}
+
+void test_transports_smart_packet__ok_pkt(void)
+{
+	/*
+	 * TODO: the trailing slash is currently mandatory. Check
+	 * if we should really enforce this. As this test is part
+	 * of a security release, I feel uneasy to change
+	 * behaviour right now and leave it for a later point.
+	 */
+	assert_pkt_fails("0007ok\n");
+	assert_ok_parses("0008ok \n", "");
+	assert_ok_parses("0009ok x\n", "x");
+	assert_pkt_fails("0013OK ref/foo/bar\n");
+	assert_ok_parses("0013ok ref/foo/bar\n", "ref/foo/bar");
+	assert_ok_parses("0012ok ref/foo/bar\n", "ref/foo/bar");
+}
+
+void test_transports_smart_packet__ng_pkt(void)
+{
+	/* TODO: same as for ok pkt */
+	assert_pkt_fails("0007ng\n");
+	assert_pkt_fails("0008ng \n");
+	assert_pkt_fails("000Bng ref\n");
+	assert_pkt_fails("000Bng ref\n");
+	/* TODO: is this a valid packet line? Probably not. */
+	assert_ng_parses("000Ang  x\n", "", "x");
+	assert_ng_parses("000Fng ref msg\n", "ref", "msg");
+	assert_ng_parses("000Fng ref msg\n", "ref", "msg");
+}
+
+void test_transports_smart_packet__unpack_pkt(void)
+{
+	assert_unpack_parses("000Dunpack ok", 1);
+	assert_unpack_parses("000Dunpack ng error-msg", 0);
+	/* TODO: the following tests should fail */
+	assert_unpack_parses("000Aunpack", 0);
+	assert_unpack_parses("0011unpack foobar", 0);
+	assert_unpack_parses("0010unpack ng ok", 0);
+	assert_unpack_parses("0010unpack okfoo", 1);
+}
+
+void test_transports_smart_packet__ref_pkt(void)
+{
+	assert_pkt_fails("002C0000000000000000000000000000000000000000");
+	assert_pkt_fails("002D0000000000000000000000000000000000000000\n");
+	assert_pkt_fails("00300000000000000000000000000000000000000000HEAD");
+	assert_pkt_fails("004800000000x0000000000000000000000000000000 refs/heads/master\0multi_ack");
+	assert_ref_parses(
+		"003F0000000000000000000000000000000000000000 refs/heads/master\0",
+		"0000000000000000000000000000000000000000", "refs/heads/master", "");
+	assert_ref_parses(
+		"00480000000000000000000000000000000000000000 refs/heads/master\0multi_ack",
+		"0000000000000000000000000000000000000000", "refs/heads/master", "multi_ack");
+	assert_ref_parses(
+		"00460000000000000000000000000000000000000000 refs/heads/master\0one two",
+		"0000000000000000000000000000000000000000", "refs/heads/master", "one two");
+	assert_ref_parses(
+		"00310000000000000000000000000000000000000000 HEAD",
+		"0000000000000000000000000000000000000000", "HEAD", NULL);
+	assert_pkt_fails("0031000000000000000000000000000000000000000 HEAD");
+	assert_ref_parses(
+		"00360000000000000000000000000000000000000000 HEAD HEAD",
+		"0000000000000000000000000000000000000000", "HEAD HEAD", NULL);
+}

From a7e87dd5e5251da3e978dcaee3ba5ff92ace1700 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 10:36:44 +0200
Subject: [PATCH 12/28] smart_pkt: honor line length when determining packet
 type

When we parse the packet type of an incoming packet line, we do not
verify that we don't overflow the provided line buffer. Fix this by
using `git__prefixncmp` instead and passing in `len`. As we have
previously already verified that `len <= linelen`, we thus won't ever
overflow the provided buffer length.

(cherry picked from commit 4a5804c983317100eed509537edc32d69c8d7aa2)
---
 src/transports/smart_pkt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 9b7e3bc4ed6..bb79edc3dde 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -459,19 +459,19 @@ int git_pkt_parse_line(
 		ret = sideband_progress_pkt(head, line, len);
 	else if (*line == GIT_SIDE_BAND_ERROR)
 		ret = sideband_error_pkt(head, line, len);
-	else if (!git__prefixcmp(line, "ACK"))
+	else if (!git__prefixncmp(line, len, "ACK"))
 		ret = ack_pkt(head, line, len);
-	else if (!git__prefixcmp(line, "NAK"))
+	else if (!git__prefixncmp(line, len, "NAK"))
 		ret = nak_pkt(head);
-	else if (!git__prefixcmp(line, "ERR "))
+	else if (!git__prefixncmp(line, len, "ERR "))
 		ret = err_pkt(head, line, len);
 	else if (*line == '#')
 		ret = comment_pkt(head, line, len);
-	else if (!git__prefixcmp(line, "ok"))
+	else if (!git__prefixncmp(line, len, "ok"))
 		ret = ok_pkt(head, line, len);
-	else if (!git__prefixcmp(line, "ng"))
+	else if (!git__prefixncmp(line, len, "ng"))
 		ret = ng_pkt(head, line, len);
-	else if (!git__prefixcmp(line, "unpack"))
+	else if (!git__prefixncmp(line, len, "unpack"))
 		ret = unpack_pkt(head, line, len);
 	else
 		ret = ref_pkt(head, line, len);

From cfb9802b462c9b918dcb9017ce3ab1d8070d1f76 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 10:46:26 +0200
Subject: [PATCH 13/28] smart_pkt: explicitly avoid integer overflows when
 parsing packets

When parsing data, progress or error packets, we need to copy the
contents of the rest of the current packet line into the flex-array of
the parsed packet. To keep track of this array's length, we then assign
the remaining length of the packet line to the structure. We do have a
mismatch of types here, as the structure's `len` field is a signed
integer, while the length that we are assigning has type `size_t`.

On nearly all platforms, this shouldn't pose any problems at all. The
line length can at most be 16^4, as the line's length is being encoded
by exactly four hex digits. But on a platforms with 16 bit integers,
this assignment could cause an overflow. While such platforms will
probably only exist in the embedded ecosystem, we still want to avoid
this potential overflow. Thus, we now simply change the structure's
`len` member to be of type `size_t` to avoid any integer promotion.

(cherry picked from commit 40fd84cca68db24f325e460a40dabe805e7a5d35)
---
 src/transports/smart.h     | 4 ++--
 src/transports/smart_pkt.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/transports/smart.h b/src/transports/smart.h
index b9f1c6fb9c9..bdf06485538 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -86,7 +86,7 @@ typedef struct {
 
 typedef struct {
 	git_pkt_type type;
-	int len;
+	size_t len;
 	char data[GIT_FLEX_ARRAY];
 } git_pkt_data;
 
@@ -94,7 +94,7 @@ typedef git_pkt_data git_pkt_progress;
 
 typedef struct {
 	git_pkt_type type;
-	int len;
+	size_t len;
 	char error[GIT_FLEX_ARRAY];
 } git_pkt_err;
 
diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index bb79edc3dde..93e608fdc57 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -118,9 +118,9 @@ static int err_pkt(git_pkt **out, const char *line, size_t len)
 	GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1);
 	pkt = git__malloc(alloclen);
 	GITERR_CHECK_ALLOC(pkt);
-
 	pkt->type = GIT_PKT_ERR;
-	pkt->len = (int)len;
+	pkt->len = len;
+
 	memcpy(pkt->error, line, len);
 	pkt->error[len] = '\0';
 
@@ -142,7 +142,7 @@ static int data_pkt(git_pkt **out, const char *line, size_t len)
 	GITERR_CHECK_ALLOC(pkt);
 
 	pkt->type = GIT_PKT_DATA;
-	pkt->len = (int) len;
+	pkt->len = len;
 	memcpy(pkt->data, line, len);
 
 	*out = (git_pkt *) pkt;
@@ -163,7 +163,7 @@ static int sideband_progress_pkt(git_pkt **out, const char *line, size_t len)
 	GITERR_CHECK_ALLOC(pkt);
 
 	pkt->type = GIT_PKT_PROGRESS;
-	pkt->len = (int) len;
+	pkt->len = len;
 	memcpy(pkt->data, line, len);
 
 	*out = (git_pkt *) pkt;

From e14dab2f0881e150b6281079eb386bd211bfd492 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 10:46:58 +0200
Subject: [PATCH 14/28] smart_pkt: check whether error packets are prefixed
 with "ERR "

In the `git_pkt_parse_line` function, we determine what kind of packet
a given packet line contains by simply checking for the prefix of that
line. Except for "ERR" packets, we always only check for the immediate
identifier without the trailing space (e.g. we check for an "ACK"
prefix, not for "ACK "). But for "ERR" packets, we do in fact include
the trailing space in our check. This is not really much of a problem at
all, but it is inconsistent with all the other packet types and thus
causes confusion when the `err_pkt` function just immediately skips the
space without checking whether it overflows the line buffer.

Adjust the check in `git_pkt_parse_line` to not include the trailing
space and instead move it into `err_pkt` for consistency.

(cherry picked from commit 786426ea6ec2a76ffe2515dc5182705fb3d44603)
---
 src/transports/smart_pkt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 93e608fdc57..52e27b90a84 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -107,10 +107,12 @@ static int comment_pkt(git_pkt **out, const char *line, size_t len)
 
 static int err_pkt(git_pkt **out, const char *line, size_t len)
 {
-	git_pkt_err *pkt;
+	git_pkt_err *pkt = NULL;
 	size_t alloclen;
 
 	/* Remove "ERR " from the line */
+	if (git__prefixncmp(line, len, "ERR "))
+		goto out_err;
 	line += 4;
 	len -= 4;
 
@@ -127,6 +129,11 @@ static int err_pkt(git_pkt **out, const char *line, size_t len)
 	*out = (git_pkt *) pkt;
 
 	return 0;
+
+out_err:
+	giterr_set(GITERR_NET, "error parsing ERR pkt-line");
+	git__free(pkt);
+	return -1;
 }
 
 static int data_pkt(git_pkt **out, const char *line, size_t len)
@@ -463,7 +470,7 @@ int git_pkt_parse_line(
 		ret = ack_pkt(head, line, len);
 	else if (!git__prefixncmp(line, len, "NAK"))
 		ret = nak_pkt(head);
-	else if (!git__prefixncmp(line, len, "ERR "))
+	else if (!git__prefixncmp(line, len, "ERR"))
 		ret = err_pkt(head, line, len);
 	else if (*line == '#')
 		ret = comment_pkt(head, line, len);

From 3fd6ce0d1eef904c3e9c1f966c9fc56a490cfd03 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 10:57:06 +0200
Subject: [PATCH 15/28] smart_pkt: adjust style of "ref" packet parsing
 function

While the function parsing ref packets doesn't have any immediately
obvious buffer overflows, it's style is different to all the other
parsing functions. Instead of checking buffer length while we go, it
does a check up-front. This causes the code to seem a lot more magical
than it really is due to some magic constants. Refactor the function to
instead make use of the style of other packet parser and verify buffer
lengths as we go.

(cherry picked from commit 5edcf5d190f3b379740b223ff6a649d08fa49581)
---
 src/transports/smart_pkt.c | 44 ++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 52e27b90a84..c9c361ad130 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -206,33 +206,25 @@ static int sideband_error_pkt(git_pkt **out, const char *line, size_t len)
  */
 static int ref_pkt(git_pkt **out, const char *line, size_t len)
 {
-	int error;
 	git_pkt_ref *pkt;
 	size_t alloclen;
 
-	if (len < GIT_OID_HEXSZ + 1) {
-		giterr_set(GITERR_NET, "error parsing pkt-line");
-		return -1;
-	}
-
-	pkt = git__malloc(sizeof(git_pkt_ref));
+	pkt = git__calloc(1, sizeof(git_pkt_ref));
 	GITERR_CHECK_ALLOC(pkt);
-
-	memset(pkt, 0x0, sizeof(git_pkt_ref));
 	pkt->type = GIT_PKT_REF;
-	if ((error = git_oid_fromstr(&pkt->head.oid, line)) < 0)
-		goto error_out;
-
-	/* Check for a bit of consistency */
-	if (line[GIT_OID_HEXSZ] != ' ') {
-		giterr_set(GITERR_NET, "error parsing pkt-line");
-		error = -1;
-		goto error_out;
-	}
 
-	/* Jump from the name */
-	line += GIT_OID_HEXSZ + 1;
-	len -= (GIT_OID_HEXSZ + 1);
+	if (len < GIT_OID_HEXSZ || git_oid_fromstr(&pkt->head.oid, line) < 0)
+		goto out_err;
+	line += GIT_OID_HEXSZ;
+	len -= GIT_OID_HEXSZ;
+
+	if (git__prefixncmp(line, len, " "))
+		goto out_err;
+	line++;
+	len--;
+
+	if (!len)
+		goto out_err;
 
 	if (line[len - 1] == '\n')
 		--len;
@@ -244,16 +236,18 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len)
 	memcpy(pkt->head.name, line, len);
 	pkt->head.name[len] = '\0';
 
-	if (strlen(pkt->head.name) < len) {
+	if (strlen(pkt->head.name) < len)
 		pkt->capabilities = strchr(pkt->head.name, '\0') + 1;
-	}
 
 	*out = (git_pkt *)pkt;
 	return 0;
 
-error_out:
+out_err:
+	giterr_set(GITERR_NET, "error parsing REF pkt-line");
+	if (pkt)
+		git__free(pkt->head.name);
 	git__free(pkt);
-	return error;
+	return -1;
 }
 
 static int ok_pkt(git_pkt **out, const char *line, size_t len)

From 82c3fc33c83587c3d496b380611d962a90c8c579 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 10:38:10 +0200
Subject: [PATCH 16/28] smart_pkt: fix buffer overflow when parsing "ACK"
 packets

We are being quite lenient when parsing "ACK" packets. First, we didn't
correctly verify that we're not overrunning the provided buffer length,
which we fix here by using `git__prefixncmp` instead of
`git__prefixcmp`. Second, we do not verify that the actual contents make
any sense at all, as we simply ignore errors when parsing the ACKs OID
and any unknown status strings. This may result in a parsed packet
structure with invalid contents, which is being silently passed to the
caller. This is being fixed by performing proper input validation and
checking of return codes.

(cherry picked from commit bc349045b1be8fb3af2b02d8554483869e54d5b8)
---
 src/transports/smart_pkt.c      | 37 ++++++++++++++++++++-------------
 tests/transports/smart/packet.c | 27 ++++++++++--------------
 2 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index c9c361ad130..9b64708bf78 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -43,34 +43,43 @@ static int flush_pkt(git_pkt **out)
 static int ack_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_ack *pkt;
-	GIT_UNUSED(line);
-	GIT_UNUSED(len);
 
 	pkt = git__calloc(1, sizeof(git_pkt_ack));
 	GITERR_CHECK_ALLOC(pkt);
-
 	pkt->type = GIT_PKT_ACK;
-	line += 3;
-	len -= 3;
 
-	if (len >= GIT_OID_HEXSZ) {
-		git_oid_fromstr(&pkt->oid, line + 1);
-		line += GIT_OID_HEXSZ + 1;
-		len -= GIT_OID_HEXSZ + 1;
-	}
+	if (git__prefixncmp(line, len, "ACK "))
+		goto out_err;
+	line += 4;
+	len -= 4;
 
-	if (len >= 7) {
-		if (!git__prefixcmp(line + 1, "continue"))
+	if (len < GIT_OID_HEXSZ || git_oid_fromstr(&pkt->oid, line) < 0)
+		goto out_err;
+	line += GIT_OID_HEXSZ;
+	len -= GIT_OID_HEXSZ;
+
+	if (len && line[0] == ' ') {
+		line++;
+		len--;
+
+		if (!git__prefixncmp(line, len, "continue"))
 			pkt->status = GIT_ACK_CONTINUE;
-		if (!git__prefixcmp(line + 1, "common"))
+		else if (!git__prefixncmp(line, len, "common"))
 			pkt->status = GIT_ACK_COMMON;
-		if (!git__prefixcmp(line + 1, "ready"))
+		else if (!git__prefixncmp(line, len, "ready"))
 			pkt->status = GIT_ACK_READY;
+		else
+			goto out_err;
 	}
 
 	*out = (git_pkt *) pkt;
 
 	return 0;
+
+out_err:
+	giterr_set(GITERR_NET, "error parsing ACK pkt-line");
+	git__free(pkt);
+	return -1;
 }
 
 static int nak_pkt(git_pkt **out)
diff --git a/tests/transports/smart/packet.c b/tests/transports/smart/packet.c
index 2b9b38f5c77..8bbb0bbcfa6 100644
--- a/tests/transports/smart/packet.c
+++ b/tests/transports/smart/packet.c
@@ -236,25 +236,20 @@ void test_transports_smart_packet__ack_pkt(void)
 			  "0000000000000000000000000000000000000000",
 			  GIT_ACK_READY);
 
-	/* TODO: these should fail as they don't have OIDs */
-	assert_ack_parses("0007ACK", "0000000000000000000000000000000000000000", 0);
-	assert_ack_parses("0008ACK ", "0000000000000000000000000000000000000000", 0);
+	/* these should fail as they don't have OIDs */
+	assert_pkt_fails("0007ACK");
+	assert_pkt_fails("0008ACK ");
 
-	/* TODO: this one is missing a space and should thus fail */
-	assert_ack_parses("0036ACK00000000000000000x0000000000000000000000 ready",
-			  "0000000000000000000000000000000000000000", 0);
+	/* this one is missing a space and should thus fail */
+	assert_pkt_fails("0036ACK00000000000000000x0000000000000000000000 ready");
 
-	/* TODO: the following ones have invalid OIDs and should thus fail */
-	assert_ack_parses("0037ACK 00000000000000000x0000000000000000000000 ready",
-			  "0000000000000000000000000000000000000000", GIT_ACK_READY);
-	assert_ack_parses("0036ACK 000000000000000000000000000000000000000 ready",
-			  "0000000000000000000000000000000000000000", 0);
-	assert_ack_parses("0036ACK 00000000000000000x0000000000000000000000ready",
-			  "0000000000000000000000000000000000000000", 0);
+	/* the following ones have invalid OIDs and should thus fail */
+	assert_pkt_fails("0037ACK 00000000000000000x0000000000000000000000 ready");
+	assert_pkt_fails("0036ACK 000000000000000000000000000000000000000 ready");
+	assert_pkt_fails("0036ACK 00000000000000000x0000000000000000000000ready");
 
-	/* TODO: this one has an invalid status and should thus fail */
-	assert_ack_parses("0036ACK 0000000000000000000000000000000000000000 read",
-			  "0000000000000000000000000000000000000000", 0);
+	/* this one has an invalid status and should thus fail */
+	assert_pkt_fails("0036ACK 0000000000000000000000000000000000000000 read");
 }
 
 void test_transports_smart_packet__nak_pkt(void)

From 8cd0a8974d2c7d6a97a1d3b944def73011660613 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 11:01:00 +0200
Subject: [PATCH 17/28] smart_pkt: fix buffer overflow when parsing "ok"
 packets

There are two different buffer overflows present when parsing "ok"
packets. First, we never verify whether the line already ends after
"ok", but directly go ahead and also try to skip the expected space
after "ok". Second, we then go ahead and use `strchr` to scan for the
terminating newline character. But in case where the line isn't
terminated correctly, this can overflow the line buffer.

Fix the issues by using `git__prefixncmp` to check for the "ok " prefix
and only checking for a trailing '\n' instead of using `memchr`. This
also fixes the issue of us always requiring a trailing '\n'.

Reported by oss-fuzz, issue 9749:

Crash Type: Heap-buffer-overflow READ {*}
Crash Address: 0x6310000389c0
Crash State:
  ok_pkt
  git_pkt_parse_line
  git_smart__store_refs

Sanitizer: address (ASAN)
(cherry picked from commit a9f1ca09178af0640963e069a2142d5ced53f0b4)
---
 src/transports/smart_pkt.c      | 21 ++++++++++++---------
 tests/transports/smart/packet.c | 11 ++++-------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 9b64708bf78..c5fa338b666 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -262,21 +262,19 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len)
 static int ok_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_ok *pkt;
-	const char *ptr;
 	size_t alloc_len;
 
 	pkt = git__malloc(sizeof(*pkt));
 	GITERR_CHECK_ALLOC(pkt);
-
 	pkt->type = GIT_PKT_OK;
 
-	line += 3; /* skip "ok " */
-	if (!(ptr = strchr(line, '\n'))) {
-		giterr_set(GITERR_NET, "invalid packet line");
-		git__free(pkt);
-		return -1;
-	}
-	len = ptr - line;
+	if (git__prefixncmp(line, len, "ok "))
+		goto out_err;
+	line += 3;
+	len -= 3;
+
+	if (line[len - 1] == '\n')
+		--len;
 
 	GITERR_CHECK_ALLOC_ADD(&alloc_len, len, 1);
 	pkt->ref = git__malloc(alloc_len);
@@ -287,6 +285,11 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len)
 
 	*out = (git_pkt *)pkt;
 	return 0;
+
+out_err:
+	giterr_set(GITERR_NET, "error parsing OK pkt-line");
+	git__free(pkt);
+	return -1;
 }
 
 static int ng_pkt(git_pkt **out, const char *line, size_t len)
diff --git a/tests/transports/smart/packet.c b/tests/transports/smart/packet.c
index 8bbb0bbcfa6..f887563f33d 100644
--- a/tests/transports/smart/packet.c
+++ b/tests/transports/smart/packet.c
@@ -280,18 +280,15 @@ void test_transports_smart_packet__comment_pkt(void)
 
 void test_transports_smart_packet__ok_pkt(void)
 {
-	/*
-	 * TODO: the trailing slash is currently mandatory. Check
-	 * if we should really enforce this. As this test is part
-	 * of a security release, I feel uneasy to change
-	 * behaviour right now and leave it for a later point.
-	 */
 	assert_pkt_fails("0007ok\n");
+	assert_ok_parses("0007ok ", "");
 	assert_ok_parses("0008ok \n", "");
+	assert_ok_parses("0008ok x", "x");
 	assert_ok_parses("0009ok x\n", "x");
+	assert_pkt_fails("001OK ref/foo/bar");
+	assert_ok_parses("0012ok ref/foo/bar", "ref/foo/bar");
 	assert_pkt_fails("0013OK ref/foo/bar\n");
 	assert_ok_parses("0013ok ref/foo/bar\n", "ref/foo/bar");
-	assert_ok_parses("0012ok ref/foo/bar\n", "ref/foo/bar");
 }
 
 void test_transports_smart_packet__ng_pkt(void)

From 02e4b27f9cc64ff41728f022c9de0ac1e0e89cbe Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 11:03:37 +0200
Subject: [PATCH 18/28] smart_pkt: fix "ng" parser accepting non-space
 character

When parsing "ng" packets, we blindly assume that the character
immediately following the "ng" prefix is a space and skip it. As the
calling function doesn't make sure that this is the case, we can thus
end up blindly accepting an invalid packet line.

Fix the issue by using `git__prefixncmp`, checking whether the line
starts with "ng ".

(cherry picked from commit b5ba7af2d30c958b090dcf135749d9afe89ec703)
---
 src/transports/smart_pkt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index c5fa338b666..c0a785d428b 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -306,9 +306,9 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len)
 
 	eol = line + len;
 
-	if (len < 3)
+	if (git__prefixncmp(line, len, "ng "))
 		goto out_err;
-	line += 3; /* skip "ng " */
+	line += 3;
 
 	if (!(ptr = memchr(line, ' ', eol - line)))
 		goto out_err;

From a8356af839f6193f4d9787c69effc961b217bb5e Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 11:04:42 +0200
Subject: [PATCH 19/28] smart_pkt: fix buffer overflow when parsing "unpack"
 packets

When checking whether an "unpack" packet returned the "ok" status or
not, we use a call to `git__prefixcmp`. In case where the passed line
isn't properly NUL terminated, though, this may overrun the line buffer.
Fix this by using `git__prefixncmp` instead.

(cherry picked from commit 5fabaca801e1f5e7a1054be612e8fabec7cd6a7f)
---
 src/transports/smart_pkt.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index c0a785d428b..2785fc6755d 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -350,13 +350,11 @@ static int unpack_pkt(git_pkt **out, const char *line, size_t len)
 {
 	git_pkt_unpack *pkt;
 
-	GIT_UNUSED(len);
-
 	pkt = git__malloc(sizeof(*pkt));
 	GITERR_CHECK_ALLOC(pkt);
-
 	pkt->type = GIT_PKT_UNPACK;
-	if (!git__prefixcmp(line, "unpack ok"))
+
+	if (!git__prefixncmp(line, len, "unpack ok"))
 		pkt->unpack_ok = 1;
 	else
 		pkt->unpack_ok = 0;

From 3bbda7d79ce02c2d70969b742a0fed124c12622c Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 11:13:59 +0200
Subject: [PATCH 20/28] smart_pkt: reorder and rename parameters of
 `git_pkt_parse_line`

The parameters of the `git_pkt_parse_line` function are quite confusing.
First, there is no real indicator what the `out` parameter is actually
all about, and it's not really clear what the `bufflen` parameter refers
to. Reorder and rename the parameters to make this more obvious.

(cherry picked from commit 0b3dfbf425d689101663341beb94237614f1b5c2)
---
 src/transports/smart.h          |  2 +-
 src/transports/smart_pkt.c      | 36 ++++++++++++++++-----------------
 src/transports/smart_protocol.c | 10 ++++-----
 tests/transports/smart/packet.c | 24 +++++++++++-----------
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/transports/smart.h b/src/transports/smart.h
index bdf06485538..ee1f4c2a8d2 100644
--- a/src/transports/smart.h
+++ b/src/transports/smart.h
@@ -184,7 +184,7 @@ int git_smart__get_push_stream(transport_smart *t, git_smart_subtransport_stream
 int git_smart__update_heads(transport_smart *t, git_vector *symrefs);
 
 /* smart_pkt.c */
-int git_pkt_parse_line(git_pkt **head, const char *line, const char **out, size_t len);
+int git_pkt_parse_line(git_pkt **head, const char **endptr, const char *line, size_t linelen);
 int git_pkt_buffer_flush(git_buf *buf);
 int git_pkt_send_flush(GIT_SOCKET s);
 int git_pkt_buffer_done(git_buf *buf);
diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 2785fc6755d..090056fd456 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -407,13 +407,13 @@ static int32_t parse_len(const char *line)
  */
 
 int git_pkt_parse_line(
-	git_pkt **head, const char *line, const char **out, size_t bufflen)
+	git_pkt **pkt, const char **endptr, const char *line, size_t linelen)
 {
 	int ret;
 	int32_t len;
 
 	/* Not even enough for the length */
-	if (bufflen > 0 && bufflen < PKT_LEN_SIZE)
+	if (linelen > 0 && linelen < PKT_LEN_SIZE)
 		return GIT_EBUFS;
 
 	len = parse_len(line);
@@ -422,7 +422,7 @@ int git_pkt_parse_line(
 		 * If we fail to parse the length, it might be because the
 		 * server is trying to send us the packfile already.
 		 */
-		if (bufflen >= 4 && !git__prefixcmp(line, "PACK")) {
+		if (linelen >= 4 && !git__prefixcmp(line, "PACK")) {
 			giterr_set(GITERR_NET, "unexpected pack file");
 		} else {
 			giterr_set(GITERR_NET, "bad packet length");
@@ -435,7 +435,7 @@ int git_pkt_parse_line(
 	 * If we were given a buffer length, then make sure there is
 	 * enough in the buffer to satisfy this line
 	 */
-	if (bufflen > 0 && bufflen < (size_t)len)
+	if (linelen > 0 && linelen < (size_t)len)
 		return GIT_EBUFS;
 
 	/*
@@ -458,36 +458,36 @@ int git_pkt_parse_line(
 	}
 
 	if (len == 0) { /* Flush pkt */
-		*out = line;
-		return flush_pkt(head);
+		*endptr = line;
+		return flush_pkt(pkt);
 	}
 
 	len -= PKT_LEN_SIZE; /* the encoded length includes its own size */
 
 	if (*line == GIT_SIDE_BAND_DATA)
-		ret = data_pkt(head, line, len);
+		ret = data_pkt(pkt, line, len);
 	else if (*line == GIT_SIDE_BAND_PROGRESS)
-		ret = sideband_progress_pkt(head, line, len);
+		ret = sideband_progress_pkt(pkt, line, len);
 	else if (*line == GIT_SIDE_BAND_ERROR)
-		ret = sideband_error_pkt(head, line, len);
+		ret = sideband_error_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ACK"))
-		ret = ack_pkt(head, line, len);
+		ret = ack_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "NAK"))
-		ret = nak_pkt(head);
+		ret = nak_pkt(pkt);
 	else if (!git__prefixncmp(line, len, "ERR"))
-		ret = err_pkt(head, line, len);
+		ret = err_pkt(pkt, line, len);
 	else if (*line == '#')
-		ret = comment_pkt(head, line, len);
+		ret = comment_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ok"))
-		ret = ok_pkt(head, line, len);
+		ret = ok_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ng"))
-		ret = ng_pkt(head, line, len);
+		ret = ng_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "unpack"))
-		ret = unpack_pkt(head, line, len);
+		ret = unpack_pkt(pkt, line, len);
 	else
-		ret = ref_pkt(head, line, len);
+		ret = ref_pkt(pkt, line, len);
 
-	*out = line + len;
+	*endptr = line + len;
 
 	return ret;
 }
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index 892287a7c47..9283a6fe2cd 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -41,7 +41,7 @@ int git_smart__store_refs(transport_smart *t, int flushes)
 
 	do {
 		if (buf->offset > 0)
-			error = git_pkt_parse_line(&pkt, buf->data, &line_end, buf->offset);
+			error = git_pkt_parse_line(&pkt, &line_end, buf->data, buf->offset);
 		else
 			error = GIT_EBUFS;
 
@@ -214,7 +214,7 @@ static int recv_pkt(git_pkt **out, git_pkt_type *pkt_type, gitno_buffer *buf)
 
 	do {
 		if (buf->offset > 0)
-			error = git_pkt_parse_line(&pkt, ptr, &line_end, buf->offset);
+			error = git_pkt_parse_line(&pkt, &line_end, ptr, buf->offset);
 		else
 			error = GIT_EBUFS;
 
@@ -748,7 +748,7 @@ static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt,
 	}
 
 	while (line_len > 0) {
-		error = git_pkt_parse_line(&pkt, line, &line_end, line_len);
+		error = git_pkt_parse_line(&pkt, &line_end, line, line_len);
 
 		if (error == GIT_EBUFS) {
 			/* Buffer the data when the inner packet is split
@@ -791,8 +791,8 @@ static int parse_report(transport_smart *transport, git_push *push)
 
 	for (;;) {
 		if (buf->offset > 0)
-			error = git_pkt_parse_line(&pkt, buf->data,
-						   &line_end, buf->offset);
+			error = git_pkt_parse_line(&pkt, &line_end,
+						   buf->data, buf->offset);
 		else
 			error = GIT_EBUFS;
 
diff --git a/tests/transports/smart/packet.c b/tests/transports/smart/packet.c
index f887563f33d..5b623a378b9 100644
--- a/tests/transports/smart/packet.c
+++ b/tests/transports/smart/packet.c
@@ -12,7 +12,7 @@ static void assert_flush_parses(const char *line)
 	const char *endptr;
 	git_pkt *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_FLUSH);
 	cl_assert_equal_strn(endptr, line + 4, linelen - 4);
 
@@ -25,7 +25,7 @@ static void assert_data_pkt_parses(const char *line, const char *expected_data,
 	const char *endptr;
 	git_pkt_data *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_DATA);
 	cl_assert_equal_i(pkt->len, expected_len);
 	cl_assert_equal_strn(pkt->data, expected_data, expected_len);
@@ -39,7 +39,7 @@ static void assert_sideband_progress_parses(const char *line, const char *expect
 	const char *endptr;
 	git_pkt_progress *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_PROGRESS);
 	cl_assert_equal_i(pkt->len, expected_len);
 	cl_assert_equal_strn(pkt->data, expected_data, expected_len);
@@ -53,7 +53,7 @@ static void assert_error_parses(const char *line, const char *expected_error, si
 	const char *endptr;
 	git_pkt_err *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_ERR);
 	cl_assert_equal_i(pkt->len, expected_len);
 	cl_assert_equal_strn(pkt->error, expected_error, expected_len);
@@ -70,7 +70,7 @@ static void assert_ack_parses(const char *line, const char *expected_oid, enum g
 
 	cl_git_pass(git_oid_fromstr(&oid, expected_oid));
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_ACK);
 	cl_assert_equal_oid(&pkt->oid, &oid);
 	cl_assert_equal_i(pkt->status, expected_status);
@@ -84,7 +84,7 @@ static void assert_nak_parses(const char *line)
 	const char *endptr;
 	git_pkt *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_NAK);
 	cl_assert_equal_strn(endptr, line + 7, linelen - 7);
 
@@ -97,7 +97,7 @@ static void assert_comment_parses(const char *line, const char *expected_comment
 	const char *endptr;
 	git_pkt_comment *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_COMMENT);
 	cl_assert_equal_strn(pkt->comment, expected_comment, strlen(expected_comment));
 
@@ -110,7 +110,7 @@ static void assert_ok_parses(const char *line, const char *expected_ref)
 	const char *endptr;
 	git_pkt_ok *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_OK);
 	cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));
 
@@ -123,7 +123,7 @@ static void assert_unpack_parses(const char *line, bool ok)
 	const char *endptr;
 	git_pkt_unpack *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_UNPACK);
 	cl_assert_equal_i(pkt->unpack_ok, ok);
 
@@ -136,7 +136,7 @@ static void assert_ng_parses(const char *line, const char *expected_ref, const c
 	const char *endptr;
 	git_pkt_ng *pkt;
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_NG);
 	cl_assert_equal_strn(pkt->ref, expected_ref, strlen(expected_ref));
 	cl_assert_equal_strn(pkt->msg, expected_msg, strlen(expected_msg));
@@ -156,7 +156,7 @@ static void assert_ref_parses_(const char *line, size_t linelen, const char *exp
 
 	cl_git_pass(git_oid_fromstr(&oid, expected_oid));
 
-	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, line, &endptr, linelen));
+	cl_git_pass(git_pkt_parse_line((git_pkt **) &pkt, &endptr, line, linelen));
 	cl_assert_equal_i(pkt->type, GIT_PKT_REF);
 	cl_assert_equal_oid(&pkt->head.oid, &oid);
 	cl_assert_equal_strn(pkt->head.name, expected_ref, strlen(expected_ref));
@@ -172,7 +172,7 @@ static void assert_pkt_fails(const char *line)
 {
 	const char *endptr;
 	git_pkt *pkt;
-	cl_git_fail(git_pkt_parse_line(&pkt, line, &endptr, strlen(line) + 1));
+	cl_git_fail(git_pkt_parse_line(&pkt, &endptr, line, strlen(line) + 1));
 }
 
 void test_transports_smart_packet__parsing_garbage_fails(void)

From 5836d8b68f8ddfe8000b11a0563a90500e139c64 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 9 Aug 2018 11:16:15 +0200
Subject: [PATCH 21/28] smart_pkt: return parsed length via out-parameter

The `parse_len` function currently directly returns the parsed length of
a packet line or an error code in case there was an error. Instead,
convert this to our usual style of using the return value as error code
only and returning the actual value via an out-parameter. Thus, we can
now convert the output parameter to an unsigned type, as the size of a
packet cannot ever be negative.

While at it, we also move the check whether the input buffer is long
enough into `parse_len` itself. We don't really want to pass around
potentially non-NUL-terminated buffers to functions without also passing
along the length, as this is dangerous in the unlikely case where other
callers for that function get added. Note that we need to make sure
though to not mess with `GIT_EBUFS` error codes, as these indicate not
an error to the caller but that he needs to fetch more data.

(cherry picked from commit c05790a8a8dd4351e61fc06c0a06c6a6fb6134dc)
---
 src/transports/smart_pkt.c | 63 ++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 090056fd456..d0f0f309e44 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -363,13 +363,17 @@ static int unpack_pkt(git_pkt **out, const char *line, size_t len)
 	return 0;
 }
 
-static int32_t parse_len(const char *line)
+static int parse_len(size_t *out, const char *line, size_t linelen)
 {
 	char num[PKT_LEN_SIZE + 1];
 	int i, k, error;
 	int32_t len;
 	const char *num_end;
 
+	/* Not even enough for the length */
+	if (linelen < PKT_LEN_SIZE)
+		return GIT_EBUFS;
+
 	memcpy(num, line, PKT_LEN_SIZE);
 	num[PKT_LEN_SIZE] = '\0';
 
@@ -390,7 +394,11 @@ static int32_t parse_len(const char *line)
 	if ((error = git__strtol32(&len, num, &num_end, 16)) < 0)
 		return error;
 
-	return len;
+	if (len < 0)
+		return -1;
+
+	*out = (size_t) len;
+	return 0;
 }
 
 /*
@@ -409,26 +417,23 @@ static int32_t parse_len(const char *line)
 int git_pkt_parse_line(
 	git_pkt **pkt, const char **endptr, const char *line, size_t linelen)
 {
-	int ret;
-	int32_t len;
-
-	/* Not even enough for the length */
-	if (linelen > 0 && linelen < PKT_LEN_SIZE)
-		return GIT_EBUFS;
+	int error;
+	size_t len;
 
-	len = parse_len(line);
-	if (len < 0) {
+	if ((error = parse_len(&len, line, linelen)) < 0) {
 		/*
-		 * If we fail to parse the length, it might be because the
-		 * server is trying to send us the packfile already.
+		 * If we fail to parse the length, it might be
+		 * because the server is trying to send us the
+		 * packfile already or because we do not yet have
+		 * enough data.
 		 */
-		if (linelen >= 4 && !git__prefixcmp(line, "PACK")) {
+		if (error == GIT_EBUFS)
+			;
+		else if (!git__prefixncmp(line, linelen, "PACK"))
 			giterr_set(GITERR_NET, "unexpected pack file");
-		} else {
+		else
 			giterr_set(GITERR_NET, "bad packet length");
-		}
-
-		return -1;
+		return error;
 	}
 
 	/*
@@ -465,31 +470,31 @@ int git_pkt_parse_line(
 	len -= PKT_LEN_SIZE; /* the encoded length includes its own size */
 
 	if (*line == GIT_SIDE_BAND_DATA)
-		ret = data_pkt(pkt, line, len);
+		error = data_pkt(pkt, line, len);
 	else if (*line == GIT_SIDE_BAND_PROGRESS)
-		ret = sideband_progress_pkt(pkt, line, len);
+		error = sideband_progress_pkt(pkt, line, len);
 	else if (*line == GIT_SIDE_BAND_ERROR)
-		ret = sideband_error_pkt(pkt, line, len);
+		error = sideband_error_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ACK"))
-		ret = ack_pkt(pkt, line, len);
+		error = ack_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "NAK"))
-		ret = nak_pkt(pkt);
+		error = nak_pkt(pkt);
 	else if (!git__prefixncmp(line, len, "ERR"))
-		ret = err_pkt(pkt, line, len);
+		error = err_pkt(pkt, line, len);
 	else if (*line == '#')
-		ret = comment_pkt(pkt, line, len);
+		error = comment_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ok"))
-		ret = ok_pkt(pkt, line, len);
+		error = ok_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "ng"))
-		ret = ng_pkt(pkt, line, len);
+		error = ng_pkt(pkt, line, len);
 	else if (!git__prefixncmp(line, len, "unpack"))
-		ret = unpack_pkt(pkt, line, len);
+		error = unpack_pkt(pkt, line, len);
 	else
-		ret = ref_pkt(pkt, line, len);
+		error = ref_pkt(pkt, line, len);
 
 	*endptr = line + len;
 
-	return ret;
+	return error;
 }
 
 void git_pkt_free(git_pkt *pkt)

From 21a2318b2335ddc416ba85cebd04236b3704839a Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 3 Oct 2018 16:17:21 +0200
Subject: [PATCH 22/28] smart_pkt: do not accept callers passing in no line
 length

Right now, we simply ignore the `linelen` parameter of
`git_pkt_parse_line` in case the caller passed in zero. But in fact, we
never want to assume anything about the provided buffer length and
always want the caller to pass in the available number of bytes.
And in fact, checking all the callers, one can see that the funciton is
never being called in case where the buffer length is zero, and thus we
are safe to remove this check.

(cherry picked from commit 1bc5b05c614c7b10de021fa392943e8e6bd12c77)
---
 src/transports/smart_pkt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index d0f0f309e44..e726d077731 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -437,10 +437,10 @@ int git_pkt_parse_line(
 	}
 
 	/*
-	 * If we were given a buffer length, then make sure there is
-	 * enough in the buffer to satisfy this line
+	 * Make sure there is enough in the buffer to satisfy
+	 * this line.
 	 */
-	if (linelen > 0 && linelen < (size_t)len)
+	if (linelen < len)
 		return GIT_EBUFS;
 
 	/*

From 232fc469e3b69db7be7bcda0a7143b62199bb956 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 5 Oct 2018 10:55:29 +0200
Subject: [PATCH 23/28] tests: always unlink created config files

While our tests in config::include create a plethora of configuration
files, most of them do not get removed at the end of each test. This can
cause weird interactions with tests that are being run at a later stage
if these later tests try to create files or directories with the same
name as any of the created configuration files.

Fix the issue by unlinking all created files at the end of these tests.

(cherry picked from commit bf662f7cf8daff2357923446cf9d22f5d4b4a66b)
---
 tests/config/include.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tests/config/include.c b/tests/config/include.c
index 0a07c9b8514..f78fe960d7c 100644
--- a/tests/config/include.c
+++ b/tests/config/include.c
@@ -32,6 +32,7 @@ void test_config_include__absolute(void)
 
 	git_buf_free(&buf);
 	git_config_free(cfg);
+	cl_git_pass(p_unlink("config-include-absolute"));
 }
 
 void test_config_include__homedir(void)
@@ -51,6 +52,8 @@ void test_config_include__homedir(void)
 	git_config_free(cfg);
 
 	cl_sandbox_set_search_path_defaults();
+
+	cl_git_pass(p_unlink("config-include-homedir"));
 }
 
 /* We need to pretend that the variables were defined where the file was included */
@@ -75,6 +78,8 @@ void test_config_include__ordering(void)
 
 	git_buf_free(&buf);
 	git_config_free(cfg);
+	cl_git_pass(p_unlink("included"));
+	cl_git_pass(p_unlink("including"));
 }
 
 /* We need to pretend that the variables were defined where the file was included */
@@ -87,8 +92,8 @@ void test_config_include__depth(void)
 
 	cl_git_fail(git_config_open_ondisk(&cfg, "a"));
 
-	p_unlink("a");
-	p_unlink("b");
+	cl_git_pass(p_unlink("a"));
+	cl_git_pass(p_unlink("b"));
 }
 
 void test_config_include__missing(void)
@@ -106,6 +111,7 @@ void test_config_include__missing(void)
 
 	git_buf_free(&buf);
 	git_config_free(cfg);
+	cl_git_pass(p_unlink("including"));
 }
 
 void test_config_include__missing_homedir(void)
@@ -126,6 +132,7 @@ void test_config_include__missing_homedir(void)
 	git_config_free(cfg);
 
 	cl_sandbox_set_search_path_defaults();
+	cl_git_pass(p_unlink("including"));
 }
 
 #define replicate10(s) s s s s s s s s s s
@@ -150,4 +157,8 @@ void test_config_include__depth2(void)
 
 	git_buf_free(&buf);
 	git_config_free(cfg);
+
+	cl_git_pass(p_unlink("top-level"));
+	cl_git_pass(p_unlink("middle"));
+	cl_git_pass(p_unlink("bottom"));
 }

From 749374314d9ebac4ccae3b2587019d41d1d89699 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 5 Oct 2018 10:56:02 +0200
Subject: [PATCH 24/28] config_file: properly ignore includes without "path"
 value

In case a configuration includes a key "include.path=" without any
value, the generated configuration entry will have its value set to
`NULL`. This is unexpected by the logic handling includes, and as soon
as we try to calculate the included path we will unconditionally
dereference that `NULL` pointer and thus segfault.

Fix the issue by returning early in both `parse_include` and
`parse_conditional_include` in case where the `file` argument is `NULL`.
Add a test to avoid future regression.

The issue has been found by the oss-fuzz project, issue 10810.

(cherry picked from commit d06d4220eec035466d1a837972a40546b8904330)
---
 src/config_file.c      |  2 +-
 tests/config/include.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/config_file.c b/src/config_file.c
index 81339b455b2..721dbef32cb 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1598,7 +1598,7 @@ static int read_on_variable(
 	result = 0;
 
 	/* Add or append the new config option */
-	if (!git__strcmp(var->entry->name, "include.path")) {
+	if (!git__strcmp(var->entry->name, "include.path") && var->entry->value) {
 		struct reader *r;
 		git_buf path = GIT_BUF_INIT;
 		char *dir;
diff --git a/tests/config/include.c b/tests/config/include.c
index f78fe960d7c..36fc59b753e 100644
--- a/tests/config/include.c
+++ b/tests/config/include.c
@@ -96,6 +96,21 @@ void test_config_include__depth(void)
 	cl_git_pass(p_unlink("b"));
 }
 
+void test_config_include__empty_path_sanely_handled(void)
+{
+	git_config *cfg;
+	git_buf buf = GIT_BUF_INIT;
+
+	cl_git_mkfile("a", "[include]\npath");
+	cl_git_pass(git_config_open_ondisk(&cfg, "a"));
+	cl_git_pass(git_config_get_string_buf(&buf, cfg, "include.path"));
+	cl_assert_equal_s("", git_buf_cstr(&buf));
+
+	git_buf_free(&buf);
+	git_config_free(cfg);
+	cl_git_pass(p_unlink("a"));
+}
+
 void test_config_include__missing(void)
 {
 	git_config *cfg;

From 7e8d978981da0140437598789436d377fc2b6758 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Fri, 5 Oct 2018 11:42:00 +0200
Subject: [PATCH 25/28] submodule: add failing test for option-injection
 protection in url and path

---
 tests/submodule/inject_option.c | 80 +++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 tests/submodule/inject_option.c

diff --git a/tests/submodule/inject_option.c b/tests/submodule/inject_option.c
new file mode 100644
index 00000000000..c13e12cf6de
--- /dev/null
+++ b/tests/submodule/inject_option.c
@@ -0,0 +1,80 @@
+#include "clar_libgit2.h"
+#include "posix.h"
+#include "path.h"
+#include "submodule_helpers.h"
+#include "fileops.h"
+#include "repository.h"
+
+static git_repository *g_repo = NULL;
+
+void test_submodule_inject_option__initialize(void)
+{
+	g_repo = setup_fixture_submodule_simple();
+}
+
+void test_submodule_inject_option__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+static int find_naughty(git_submodule *sm, const char *name, void *payload)
+{
+	int *foundit = (int *) payload;
+
+	GIT_UNUSED(sm);
+
+	if (!git__strcmp("naughty", name))
+		*foundit = true;
+
+	return 0;
+}
+
+void test_submodule_inject_option__url(void)
+{
+	int foundit;
+	git_submodule *sm;
+	git_buf buf = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_joinpath(&buf, git_repository_workdir(g_repo), ".gitmodules"));
+	cl_git_rewritefile(buf.ptr,
+			   "[submodule \"naughty\"]\n"
+			   "    path = testrepo\n"
+			   "    url = -u./payload\n");
+	git_buf_free(&buf);
+
+	/* We do want to find it, but with the appropriate field empty */
+	foundit = 0;
+	cl_git_pass(git_submodule_foreach(g_repo, find_naughty, &foundit));
+	cl_assert_equal_i(1, foundit);
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "naughty"));
+	cl_assert_equal_s("testrepo", git_submodule_path(sm));
+	cl_assert_equal_p(NULL, git_submodule_url(sm));
+
+	git_submodule_free(sm);
+}
+
+void test_submodule_inject_option__path(void)
+{
+	int foundit;
+	git_submodule *sm;
+	git_buf buf = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_joinpath(&buf, git_repository_workdir(g_repo), ".gitmodules"));
+	cl_git_rewritefile(buf.ptr,
+			   "[submodule \"naughty\"]\n"
+			   "    path = --something\n"
+			   "    url = blah.git\n");
+	git_buf_free(&buf);
+
+	/* We do want to find it, but with the appropriate field empty */
+	foundit = 0;
+	cl_git_pass(git_submodule_foreach(g_repo, find_naughty, &foundit));
+	cl_assert_equal_i(1, foundit);
+
+	cl_git_pass(git_submodule_lookup(&sm, g_repo, "naughty"));
+	cl_assert_equal_s("naughty", git_submodule_path(sm));
+	cl_assert_equal_s("blah.git", git_submodule_url(sm));
+
+	git_submodule_free(sm);
+}

From b93e82d4ca35e3be6f0dddc542fd7c847060777b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me>
Date: Fri, 5 Oct 2018 11:47:39 +0200
Subject: [PATCH 26/28] submodule: ignore path and url attributes if they look
 like options

These can be used to inject options in an implementation which performs a
recursive clone by executing an external command via crafted url and path
attributes such that it triggers a local executable to be run.

The library is not vulnerable as we do not rely on external executables but a
user of the library might be relying on that so we add this protection.

This matches this aspect of git's fix for CVE-2018-17456.
---
 src/submodule.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/submodule.c b/src/submodule.c
index 96c8e667569..74008dcf632 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1792,6 +1792,14 @@ static int get_value(const char **out, git_config *cfg, git_buf *buf, const char
 	return error;
 }
 
+static bool looks_like_command_line_option(const char *s)
+{
+	if (s && s[0] == '-')
+		return true;
+
+	return false;
+}
+
 static int submodule_read_config(git_submodule *sm, git_config *cfg)
 {
 	git_buf key = GIT_BUF_INIT;
@@ -1805,24 +1813,31 @@ static int submodule_read_config(git_submodule *sm, git_config *cfg)
 
 	if ((error = get_value(&value, cfg, &key, sm->name, "path")) == 0) {
 		in_config = 1;
+		/* We would warn here if we had that API */
+		if (!looks_like_command_line_option(value)) {
 	/*
 	 * TODO: if case insensitive filesystem, then the following strcmp
 	 * should be strcasecmp
 	 */
-		if (strcmp(sm->name, value) != 0) {
-			if (sm->path != sm->name)
-				git__free(sm->path);
-			sm->path = git__strdup(value);
-			GITERR_CHECK_ALLOC(sm->path);
+			if (strcmp(sm->name, value) != 0) {
+				if (sm->path != sm->name)
+					git__free(sm->path);
+				sm->path = git__strdup(value);
+				GITERR_CHECK_ALLOC(sm->path);
+			}
+
 		}
 	} else if (error != GIT_ENOTFOUND) {
 		goto cleanup;
 	}
 
 	if ((error = get_value(&value, cfg, &key, sm->name, "url")) == 0) {
-		in_config = 1;
-		sm->url = git__strdup(value);
-		GITERR_CHECK_ALLOC(sm->url);
+		/* We would warn here if we had that API */
+		if (!looks_like_command_line_option(value)) {
+			in_config = 1;
+			sm->url = git__strdup(value);
+			GITERR_CHECK_ALLOC(sm->url);
+		}
 	} else if (error != GIT_ENOTFOUND) {
 		goto cleanup;
 	}

From b1d39682764f1535aabdb47ec789575b76aad6df Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 6 Sep 2018 13:14:19 +0200
Subject: [PATCH 27/28] CHANGELOG: update for v0.26.7

---
 CHANGELOG.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e51f7627140..a4903d39247 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,50 @@
+v0.26.7
+-------
+
+This is a security release fixing the following list of issues:
+
+- Submodule URLs and paths with a leading "-" are now ignored.
+  This is due to the recently discovered CVE-2018-17456, which
+  can lead to arbitrary code execution in upstream git. While
+  libgit2 itself is not vulnerable, it can be used to inject
+  options in an implementation which performs a recursive clone
+  by executing an external command.
+
+- When running repack while doing repo writes,
+  `packfile_load__cb()` could see some temporary files in the
+  directory that were bigger than the usual, and makes `memcmp`
+  overflow on the `p->pack_name` string. This issue was reported
+  and fixed by bisho.
+
+- The configuration file parser used unbounded recursion to parse
+  multiline variables, which could lead to a stack overflow. The
+  issue was reported by the oss-fuzz project, issue 10048 and
+  fixed by Nelson Elhage.
+
+- The fix to the unbounded recursion introduced a memory leak in
+  the config parser. While this leak was never in a public
+  release, the oss-fuzz project reported this as issue 10127. The
+  fix was implemented by Nelson Elhage and Patrick Steinhardt.
+
+- When parsing "ok" packets received via the smart protocol, our
+  parsing code did not correctly verify the bounds of the
+  packets, which could result in a heap-buffer overflow. The
+  issue was reported by the oss-fuzz project, issue 9749 and
+  fixed by Patrick Steinhardt.
+
+- The parsing code for the smart protocol has been tightened in
+  general, fixing heap-buffer overflows when parsing the packet
+  type as well as for "ACK" and "unpack" packets. The issue was
+  discovered and fixed by Patrick Steinhardt.
+
+- Fixed potential integer overflows on platforms with 16 bit
+  integers when parsing packets for the smart protocol. The issue
+  was discovered and fixed by Patrick Steinhardt.
+
+- Fixed potential NULL pointer dereference when parsing
+  configuration files which have "include.path" statements
+  without a value.
+
 v0.26.6
 -------
 

From 9102156c83d5e9f443ab1f3c00e55c5c3041a4d0 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 6 Sep 2018 13:14:40 +0200
Subject: [PATCH 28/28] version: raise to v0.26.7

---
 include/git2/version.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/git2/version.h b/include/git2/version.h
index f16bb911726..520726ec551 100644
--- a/include/git2/version.h
+++ b/include/git2/version.h
@@ -7,10 +7,10 @@
 #ifndef INCLUDE_git_version_h__
 #define INCLUDE_git_version_h__
 
-#define LIBGIT2_VERSION "0.26.6"
+#define LIBGIT2_VERSION "0.26.7"
 #define LIBGIT2_VER_MAJOR 0
 #define LIBGIT2_VER_MINOR 26
-#define LIBGIT2_VER_REVISION 6
+#define LIBGIT2_VER_REVISION 7
 #define LIBGIT2_VER_PATCH 0
 
 #define LIBGIT2_SOVERSION 26