Fix bug #70019 - limit extracted files to given directory

diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index 8cfe0c82284a5c4dcbb5f2bf520760390f279a0c..b65218186973df6920e1dbe1331f05987dd58c7c 100644 (file)
--- 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 (file)
index 0000000..d7976a1
--- /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 (file)
index 0000000..faf152d
Binary files /dev/null and b/ext/phar/tests/bug70019.zip differ