Sanitize directory paths and do not allow '..'

Archived files with '..' in the directory path might be used to escape
the directory being extracted into and overwrite arbitrary files.

lib/ext_header.c

@@ -123,6 +123,7 @@ static int ext_header_filename_decoder(LHAFileHeader *header,
                                        size_t data_len)
 {
 	char *new_filename;
+	unsigned int i;
 
 	new_filename = malloc(data_len + 1);
 
@@ -133,6 +134,16 @@ static int ext_header_filename_decoder(LHAFileHeader *header,
 	memcpy(new_filename, data, data_len);
 	new_filename[data_len] = '\0';
 
+	// Sanitize the filename that was read. It is not allowed to
+	// contain a path separator, which could potentially be used
+	// to do something malicious.
+
+	for (i = 0; new_filename[i] != '\0'; ++i) {
+		if (new_filename[i] == '/') {
+			new_filename[i] = '_';
+		}
+	}
+
 	free(header->filename);
 	header->filename = new_filename;
 

lib/lha_file_header.c

@@ -789,6 +789,81 @@ static int decode_level3_header(LHAFileHeader **header, LHAInputStream *stream)
 	return 1;
 }
 
+
+// "Collapse" a path down, by removing all instances of "." and ".."
+// paths. This is to protect against malicious archives that might include
+// ".." in a path to break out of the extract directory.
+
+static void collapse_path(char *filename)
+{
+	unsigned int currpath_len;
+	char *currpath;
+	char *r, *w;
+
+	// If the path starts with a /, it is an absolute path; skip over
+	// that first character and don't remove it.
+
+	if (filename[0] == '/') {
+		++filename;
+	}
+
+	// Step through each character, copying it from 'r' to 'w'. It
+	// is always the case that w <= r, and the final string will
+	// be equal in length or shorter than the original.
+
+	currpath = filename;
+	w = filename;
+
+	for (r = filename; *r != '\0'; ++r) {
+		*w++ = *r;
+
+		// Each time a new path separator is found, examine the
+		// path that was just written.
+
+		if (*r == '/') {
+
+			currpath_len = w - currpath - 1;
+
+			// Empty path (//) or current directory (.)?
+
+			if (currpath_len == 0
+			 || (currpath_len == 1 && currpath[0] == '.')) {
+				w = currpath;
+
+			// Parent directory (..)?
+
+			} else if (currpath_len == 2
+			        && currpath[0] == '.' && currpath[1] == '.') {
+
+				// Walk back up by one directory. Don't go
+				// past the start of the string.
+
+				if (currpath == filename) {
+					w = filename;
+				} else {
+					w = currpath - 1;
+
+					while (w > filename) {
+						if (*(w - 1) == '/') {
+							break;
+						}
+						--w;
+					}
+
+					currpath = w;
+				}
+
+			// Save for next time we start a new path.
+
+			} else {
+				currpath = w;
+			}
+		}
+	}
+
+	*w = '\0';
+}
+
 LHAFileHeader *lha_file_header_read(LHAInputStream *stream)
 {
 	LHAFileHeader *header;
@@ -889,6 +964,12 @@ LHAFileHeader *lha_file_header_read(LHAInputStream *stream)
 		fix_msdos_allcaps(header);
 	}
 
+	// Collapse special directory paths to ensure the path is clean.
+
+	if (header->path != NULL) {
+		collapse_path(header->path);
+	}
+
 	// Is this header generated by OS-9/68k LHA? If so, any Unix
 	// permissions are actually OS-9 permissions.
 

test/archives/regression/README

@@ -29,3 +29,8 @@ symlink1.lzh        - Symbolic link test. The archive contains a file named
                       and then as an actual file. Extraction should not
                       create bar.txt, or arbitrary files could be overwritten
                       by a maliciously constructed archive file.
+
+dotdot.lzh          - Archive containing paths with '..'. This could be used
+                      to break out of the extract directory and overwrite
+                      arbitrary files on the filesystem.
+

test/output/regression/dotdot.lzh-e.txt

@@ -0,0 +1,2 @@
+evil1.txt	- Melting  :  .evil1.txt	- Melting  :  oevil1.txt	- Melted  
+evil2.txt	- Melting  :  .evil2.txt	- Melting  :  oevil2.txt	- Melted  

test/test-extract

@@ -776,6 +776,23 @@ test_symlink() {
 	remove_sandboxes
 }
 
+# '..' path check. It should not be possible to escape the extract
+# directory.
+
+test_dotdot() {
+	local archive_file=regression/dotdot.lzh
+
+	make_sandboxes
+
+	lha_check_output output/regression/dotdot.lzh-e.txt \
+	    ef $(test_arc_file "$archive_file")
+
+	check_not_exists "$archive_file" "../evil1.txt"
+	check_not_exists "$archive_file" "../evil2.txt"
+
+	remove_sandboxes
+}
+
 test_overwrite_prompt
 test_overwrite_all a
 test_overwrite_all A
@@ -788,4 +805,5 @@ test_extract_list
 test_extract_truncated
 
 test_symlink symlink1.lzh
+test_dotdot