Merge pull request #4835 from pks-t/pks/v0.26.7

Security release v0.26.7

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
 -------
 

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

src/config_file.c

@@ -1347,48 +1347,50 @@ 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;
+	bool multiline = true;
+
+	while (multiline) {
+		char *line = NULL, *proc_line = NULL;
+		int error;
+
+		/* Check that the next line exists */
+		line = reader_readline(reader, false);
+		GITERR_CHECK_ALLOC(line);
+
+		/*
+		 * We've reached the end of the file, there is no continuation.
+		 * (this is not an error).
+		 */
+		if (line[0] == '\0') {
+			error = 0;
+			goto out;
+		}
 
-	/* Check that the next line exists */
-	line = reader_readline(reader, false);
-	if (line == NULL)
-		return -1;
+		/* If it was just a comment, pretend it didn't exist */
+		quote_count = strip_comments(line, !!in_quotes);
+		if (line[0] == '\0')
+			goto next;
 
-	/* 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 ((error = unescape_line(&proc_line, &multiline,
+					   line, in_quotes)) < 0)
+			goto out;
 
-	quote_count = strip_comments(line, !!in_quotes);
+		/* Add this line to the multiline var */
+		if ((error = git_buf_puts(value, proc_line)) < 0)
+			goto out;
 
-	/* If it was just a comment, pretend it didn't exist */
-	if (line[0] == '\0') {
+next:
 		git__free(line);
-		return parse_multiline_variable(reader, value, quote_count);
-		/* TODO: unbounded recursion. This **could** be exploitable */
-	}
+		git__free(proc_line);
+		in_quotes = quote_count;
+		continue;
 
-	if (unescape_line(&proc_line, &multiline, line, in_quotes) < 0) {
+out:
 		git__free(line);
-		return -1;
+		git__free(proc_line);
+		return error;
 	}
-	/* 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;
 }
@@ -1596,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;

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;
 	}
 

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;
 	}

src/transports/smart.h

@@ -28,22 +28,22 @@
 
 extern bool git_smart__ofs_delta_enabled;
 
-enum git_pkt_type {
+typedef enum {
 	GIT_PKT_CMD,
 	GIT_PKT_FLUSH,
 	GIT_PKT_REF,
 	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,
 	GIT_PKT_PROGRESS,
 	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,62 +55,62 @@ 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;
 };
 
 /* 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;
-	int len;
+	git_pkt_type type;
+	size_t len;
 	char data[GIT_FLEX_ARRAY];
 } git_pkt_data;
 
 typedef git_pkt_data git_pkt_progress;
 
 typedef struct {
-	enum git_pkt_type type;
-	int len;
+	git_pkt_type type;
+	size_t 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;
 
@@ -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);