Blob Blame History Raw
From f667f381486371ccb19f2b1e7307a695cf3a5279 Mon Sep 17 00:00:00 2001
From: Tim Kientzle <kientzle@acm.org>
Date: Sun, 3 Apr 2016 11:03:22 -0700
Subject: [PATCH] Issue 550: Fix out-of-bounds read in mtree.

The mtree parser scanned from the end of the string to identify
the filename when the filename is the last element of the line.
If the filename was the entire line, the logic would scan back
to before the start of the string.

The revised logic scans from the beginning of the string
and remembers the last separator position to locate the
trailing filename.
---
 libarchive/archive_read_support_format_mtree.c | 53 ++++++++++++++------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/libarchive/archive_read_support_format_mtree.c b/libarchive/archive_read_support_format_mtree.c
index 397bfee..a672cb3 100644
--- a/libarchive/archive_read_support_format_mtree.c
+++ b/libarchive/archive_read_support_format_mtree.c
@@ -826,8 +826,8 @@ process_add_entry(struct archive_read *a, struct mtree *mtree,
 	struct mtree_entry *entry;
 	struct mtree_option *iter;
 	const char *next, *eq, *name, *end;
-	size_t len;
-	int r;
+	size_t name_len, len;
+	int r, i;
 
 	if ((entry = malloc(sizeof(*entry))) == NULL) {
 		archive_set_error(&a->archive, errno, "Can't allocate memory");
@@ -847,43 +847,48 @@ process_add_entry(struct archive_read *a, struct mtree *mtree,
 	*last_entry = entry;
 
 	if (is_form_d) {
-		/*
-		 * This form places the file name as last parameter.
-		 */
-		name = line + line_len -1;
+		/* Filename is last item on line. */
+		/* Adjust line_len to trim trailing whitespace */
 		while (line_len > 0) {
-			if (*name != '\r' && *name != '\n' &&
-			    *name != '\t' && *name != ' ')
+			char last_character = line[line_len - 1];
+			if (last_character == '\r'
+			    || last_character == '\n'
+			    || last_character == '\t'
+			    || last_character == ' ') {
+				line_len--;
+			} else {
 				break;
-			name--;
-			line_len--;
+			}
 		}
-		len = 0;
-		while (line_len > 0) {
-			if (*name == '\r' || *name == '\n' ||
-			    *name == '\t' || *name == ' ') {
-				name++;
-				break;
+		/* Name starts after the last whitespace separator */
+		name = line;
+		for (i = 0; i < line_len; i++) {
+			if (line[i] == '\r'
+			    || line[i] == '\n'
+			    || line[i] == '\t'
+			    || line[i] == ' ') {
+				name = line + i + 1;
 			}
-			name--;
-			line_len--;
-			len++;
 		}
+		name_len = line + line_len - name;
 		end = name;
 	} else {
-		len = strcspn(line, " \t\r\n");
+		/* Filename is first item on line */
+		name_len = strcspn(line, " \t\r\n");
 		name = line;
-		line += len;
+		line += name_len;
 		end = line + line_len;
 	}
+	/* name/name_len is the name within the line. */
+	/* line..end brackets the entire line except the name */
 
-	if ((entry->name = malloc(len + 1)) == NULL) {
+	if ((entry->name = malloc(name_len + 1)) == NULL) {
 		archive_set_error(&a->archive, errno, "Can't allocate memory");
 		return (ARCHIVE_FATAL);
 	}
 
-	memcpy(entry->name, name, len);
-	entry->name[len] = '\0';
+	memcpy(entry->name, name, name_len);
+	entry->name[name_len] = '\0';
 	parse_escapes(entry->name, entry);
 
 	for (iter = *global; iter != NULL; iter = iter->next) {
-- 
2.7.4