Fix bug #70019 - limit extracted files to given directory

3 files changed, 68 insertions, 4 deletions

diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index 8cfe0c8228..b652181869 100644
--- a/ext/phar/phar_object.c
+++ b/ext/phar/phar_object.c
@@ -4200,6 +4200,9 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char *
 	char *fullpath;
 	const char *slash;
 	mode_t mode;
+	cwd_state new_state;
+	char *filename;
+	size_t filename_len;
 
 	if (entry->is_mounted) {
 		/* silently ignore mounted entries */
@@ -4209,8 +4212,39 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char *
 	if (entry->filename_len >= sizeof(".phar")-1 && !memcmp(entry->filename, ".phar", sizeof(".phar")-1)) {
 		return SUCCESS;
 	}
+	/* strip .. from path and restrict it to be under dest directory */
+	new_state.cwd = (char*)malloc(2);
+	new_state.cwd[0] = DEFAULT_SLASH;
+	new_state.cwd[1] = '\0';
+	new_state.cwd_length = 1;
+	if (virtual_file_ex(&new_state, entry->filename, NULL, CWD_EXPAND TSRMLS_CC) != 0 ||
+			new_state.cwd_length <= 1) {
+		if (EINVAL == errno && entry->filename_len > 50) {
+			char *tmp = estrndup(entry->filename, 50);
+			spprintf(error, 4096, "Cannot extract \"%s...\" to \"%s...\", extracted filename is too long for filesystem", tmp, dest);
+			efree(tmp);
+		} else {
+			spprintf(error, 4096, "Cannot extract \"%s\", internal error", entry->filename);
+		}
+		free(new_state.cwd);
+		return FAILURE;
+	}
+	filename = new_state.cwd + 1;
+	filename_len = new_state.cwd_length - 1;
+#ifdef PHP_WIN32
+	/* unixify the path back, otherwise non zip formats might be broken */
+	{
+		int cnt = filename_len;
+
+		do {
+			if ('\\' == filename[cnt]) {
+				filename[cnt] = '/';
+			}
+		} while (cnt-- >= 0);
+	}
+#endif
 
-	len = spprintf(&fullpath, 0, "%s/%s", dest, entry->filename);
+	len = spprintf(&fullpath, 0, "%s/%s", dest, filename);
 
 	if (len >= MAXPATHLEN) {
 		char *tmp;
@@ -4224,18 +4258,21 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char *
 			spprintf(error, 4096, "Cannot extract \"%s\" to \"%s...\", extracted filename is too long for filesystem", entry->filename, fullpath);
 		}
 		efree(fullpath);
+		free(new_state.cwd);
 		return FAILURE;
 	}
 
 	if (!len) {
 		spprintf(error, 4096, "Cannot extract \"%s\", internal error", entry->filename);
 		efree(fullpath);
+		free(new_state.cwd);
 		return FAILURE;
 	}
 
 	if (PHAR_OPENBASEDIR_CHECKPATH(fullpath)) {
 		spprintf(error, 4096, "Cannot extract \"%s\" to \"%s\", openbasedir/safe mode restrictions in effect", entry->filename, fullpath);
 		efree(fullpath);
+		free(new_state.cwd);
 		return FAILURE;
 	}
 
@@ -4243,14 +4280,15 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char *
 	if (!overwrite && SUCCESS == php_stream_stat_path(fullpath, &ssb)) {
 		spprintf(error, 4096, "Cannot extract \"%s\" to \"%s\", path already exists", entry->filename, fullpath);
 		efree(fullpath);
+		free(new_state.cwd);
 		return FAILURE;
 	}
 
 	/* perform dirname */
-	slash = zend_memrchr(entry->filename, '/', entry->filename_len);
+	slash = zend_memrchr(filename, '/', filename_len);
 
 	if (slash) {
-		fullpath[dest_len + (slash - entry->filename) + 1] = '\0';
+		fullpath[dest_len + (slash - filename) + 1] = '\0';
 	} else {
 		fullpath[dest_len] = '\0';
 	}
@@ -4260,23 +4298,27 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char *
 			if (!php_stream_mkdir(fullpath, entry->flags & PHAR_ENT_PERM_MASK,  PHP_STREAM_MKDIR_RECURSIVE, NULL)) {
 				spprintf(error, 4096, "Cannot extract \"%s\", could not create directory \"%s\"", entry->filename, fullpath);
 				efree(fullpath);
+				free(new_state.cwd);
 				return FAILURE;
 			}
 		} else {
 			if (!php_stream_mkdir(fullpath, 0777,  PHP_STREAM_MKDIR_RECURSIVE, NULL)) {
 				spprintf(error, 4096, "Cannot extract \"%s\", could not create directory \"%s\"", entry->filename, fullpath);
 				efree(fullpath);
+				free(new_state.cwd);
 				return FAILURE;
 			}
 		}
 	}
 
 	if (slash) {
-		fullpath[dest_len + (slash - entry->filename) + 1] = '/';
+		fullpath[dest_len + (slash - filename) + 1] = '/';
 	} else {
 		fullpath[dest_len] = '/';
 	}
 
+	filename = NULL;
+	free(new_state.cwd);
 	/* it is a standalone directory, job done */
 	if (entry->is_dir) {
 		efree(fullpath);
diff --git a/ext/phar/tests/bug70019.phpt b/ext/phar/tests/bug70019.phpt
new file mode 100644
index 0000000000..d7976a180d
--- /dev/null
+++ b/ext/phar/tests/bug70019.phpt
@@ -0,0 +1,22 @@
+--TEST--
+Bug #70019 Files extracted from archive may be placed outside of destination directory
+--FILE--
+<?php
+$dir = __DIR__."/bug70019";
+$phar = new PharData(__DIR__."/bug70019.zip");
+if(!is_dir($dir)) {
+  mkdir($dir);
+}
+$phar->extractTo($dir); 
+var_dump(file_exists("$dir/ThisIsATestFile.txt"));
+?>
+===DONE===
+--CLEAN--
+<?php
+$dir = __DIR__."/bug70019";
+unlink("$dir/ThisIsATestFile.txt");
+rmdir($dir);
+?>
+--EXPECTF--
+bool(true)
+===DONE===
diff --git a/ext/phar/tests/bug70019.zip b/ext/phar/tests/bug70019.zip
new file mode 100644
index 0000000000..faf152df7e
--- /dev/null
+++ b/ext/phar/tests/bug70019.zip