Fix VirtualProtect() related Phar issues

We must not (try to) modify shared values, but rather have to use our
own copies, if unixified filenames are required on Windows.  To avoid
excessive string duplication, we add checks whether the filenames are
already unixified (i.e. do not contain backslashes).  To improve the
performance if we need to copy strings, we use do_alloca() and friends.

Besides generally being somewhat messy, the handling of unixified
filenames is still suboptimal performance-wise, but we leave this for a
future cleanup, and focus on fixing the issue at hand for now.

We also enable opcache.protect_memory for the AppVeyor CI.

diff --git a/appveyor/test_task.bat b/appveyor/test_task.bat
index 6bf99ad23ea8b8b77774843acaa567efb0bcac6f..6588ce193ccd2152d1fa41cb2da7c6bda3a249a3 100644 (file)
--- a/appveyor/test_task.bat
+++ b/appveyor/test_task.bat
@@ -58,7 +58,7 @@ rem set OPENSSL_CONF=
 rem set SSLEAY_CONF=
 
 rem prepare for Opcache
-if "%OPCACHE%" equ "1" set OPCACHE_OPTS=-d opcache.enabled=1 -d opcache.enable_cli=1
+if "%OPCACHE%" equ "1" set OPCACHE_OPTS=-d opcache.enabled=1 -d opcache.enable_cli=1 -d opcache.protect_memory=1
 
 rem prepare for enchant
 mkdir c:\enchant_plugins

diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index 7cd33d2197aef3ed52ae1e1f1f2f338f1ad3ea12..25bb2c9e4af8d507ceb3c259a8cb3e33e08f18e1 100644 (file)
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -533,28 +533,32 @@ int phar_open_parsed_phar(char *fname, size_t fname_len, char *alias, size_t ali
 {
        phar_archive_data *phar;
 #ifdef PHP_WIN32
-       char *unixfname;
+       char *save_fname;
+       ALLOCA_FLAG(fname_use_heap)
 #endif
 
        if (error) {
                *error = NULL;
        }
 #ifdef PHP_WIN32
-       unixfname = estrndup(fname, fname_len);
-       phar_unixify_path_separators(unixfname, fname_len);
-
-       if (SUCCESS == phar_get_archive(&phar, unixfname, fname_len, alias, alias_len, error)
-               && ((alias && fname_len == phar->fname_len
-               && !strncmp(unixfname, phar->fname, fname_len)) || !alias)
-       ) {
-               phar_entry_info *stub;
-               efree(unixfname);
-#else
+       save_fname = fname;
+       if (memchr(fname, '\\', fname_len)) {
+               fname = do_alloca(fname_len + 1, fname_use_heap);
+               memcpy(fname, save_fname, fname_len);
+               fname[fname_len] = '\0';
+               phar_unixify_path_separators(fname, fname_len);
+       }
+#endif
        if (SUCCESS == phar_get_archive(&phar, fname, fname_len, alias, alias_len, error)
                && ((alias && fname_len == phar->fname_len
                && !strncmp(fname, phar->fname, fname_len)) || !alias)
        ) {
                phar_entry_info *stub;
+#ifdef PHP_WIN32
+               if (fname != save_fname) {
+                       free_alloca(fname, fname_use_heap);
+                       fname = save_fname;
+               }
 #endif
                /* logic above is as follows:
                   If an explicit alias was requested, ensure the filename passed in
@@ -581,7 +585,10 @@ int phar_open_parsed_phar(char *fname, size_t fname_len, char *alias, size_t ali
                return SUCCESS;
        } else {
 #ifdef PHP_WIN32
-               efree(unixfname);
+               if (fname != save_fname) {
+                       free_alloca(fname, fname_use_heap);
+                       fname = save_fname;
+               }
 #endif
                if (pphar) {
                        *pphar = NULL;
@@ -2229,8 +2236,10 @@ int phar_split_fname(const char *filename, size_t filename_len, char **arch, siz
        ext_len = 0;
 #ifdef PHP_WIN32
        save = (char *)filename;
-       filename = estrndup(filename, filename_len);
-       phar_unixify_path_separators((char *)filename, filename_len);
+       if (memchr(filename, '\\', filename_len)) {
+               filename = estrndup(filename, filename_len);
+               phar_unixify_path_separators((char *)filename, filename_len);
+       }
 #endif
        if (phar_detect_phar_fname_ext(filename, filename_len, &ext_str, &ext_len, executable, for_create, 0) == FAILURE) {
                if (ext_len != -1) {
@@ -2244,7 +2253,9 @@ int phar_split_fname(const char *filename, size_t filename_len, char **arch, siz
                        }
 
 #ifdef PHP_WIN32
-                       efree((char *)filename);
+                       if (filename != save) {
+                               efree((char *)filename);
+                       }
 #endif
                        return FAILURE;
                }
@@ -2269,7 +2280,9 @@ int phar_split_fname(const char *filename, size_t filename_len, char **arch, siz
        }
 
 #ifdef PHP_WIN32
-       efree((char *)filename);
+       if (filename != save) {
+               efree((char *)filename);
+       }
 #endif
 
        return SUCCESS;

diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index f2d1341047287ca377929453c6922d1e4d4931ae..72c7c4a535e8e609a7f0866628d12f01777a966a 100644 (file)
--- a/ext/phar/phar_object.c
+++ b/ext/phar/phar_object.c
@@ -438,6 +438,10 @@ PHP_METHOD(Phar, mount)
        size_t fname_len, arch_len, entry_len;
        size_t path_len, actual_len;
        phar_archive_data *pphar;
+#ifdef PHP_WIN32
+       char *save_fname;
+       ALLOCA_FLAG(fname_use_heap)
+#endif
 
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "pp", &path, &path_len, &actual, &actual_len) == FAILURE) {
                return;
@@ -447,7 +451,13 @@ PHP_METHOD(Phar, mount)
        fname_len = strlen(fname);
 
 #ifdef PHP_WIN32
-       phar_unixify_path_separators(fname, fname_len);
+       save_fname = fname;
+       if (memchr(fname, '\\', fname_len)) {
+               fname = do_alloca(fname_len + 1, fname_use_heap);
+               memcpy(fname, save_fname, fname_len);
+               fname[fname_len] = '\0';
+               phar_unixify_path_separators(fname, fname_len);
+       }
 #endif
 
        if (fname_len > 7 && !memcmp(fname, "phar://", 7) && SUCCESS == phar_split_fname(fname, fname_len, &arch, &arch_len, &entry, &entry_len, 2, 0)) {
@@ -457,7 +467,7 @@ PHP_METHOD(Phar, mount)
                if (path_len > 7 && !memcmp(path, "phar://", 7)) {
                        zend_throw_exception_ex(phar_ce_PharException, 0, "Can only mount internal paths within a phar archive, use a relative path instead of \"%s\"", path);
                        efree(arch);
-                       return;
+                       goto finish;
                }
 carry_on2:
                if (NULL == (pphar = zend_hash_str_find_ptr(&(PHAR_G(phar_fname_map)), arch, arch_len))) {
@@ -472,7 +482,8 @@ carry_on2:
                        if (arch) {
                                efree(arch);
                        }
-                       return;
+
+                       goto finish;
                }
 carry_on:
                if (SUCCESS != phar_mount_entry(pphar, actual, actual_len, path, path_len)) {
@@ -485,7 +496,7 @@ carry_on:
                                efree(arch);
                        }
 
-                       return;
+                       goto finish;
                }
 
                if (entry && path && path == entry) {
@@ -496,7 +507,7 @@ carry_on:
                        efree(arch);
                }
 
-               return;
+               goto finish;
        } else if (HT_IS_INITIALIZED(&PHAR_G(phar_fname_map)) && NULL != (pphar = zend_hash_str_find_ptr(&(PHAR_G(phar_fname_map)), fname, fname_len))) {
                goto carry_on;
        } else if (PHAR_G(manifest_cached) && NULL != (pphar = zend_hash_str_find_ptr(&cached_phars, fname, fname_len))) {
@@ -512,6 +523,14 @@ carry_on:
        }
 
        zend_throw_exception_ex(phar_ce_PharException, 0, "Mounting of %s to %s failed", path, actual);
+
+finish: ;
+#ifdef PHP_WIN32
+       if (fname != save_fname) {
+               free_alloca(fname, fname_use_heap);
+               fname = save_fname;
+       }
+#endif
 }
 /* }}} */
 
@@ -570,8 +589,10 @@ PHP_METHOD(Phar, webPhar)
        }
 
 #ifdef PHP_WIN32
-       fname = estrndup(fname, fname_len);
-       phar_unixify_path_separators(fname, fname_len);
+       if (memchr(fname, '\\', fname_len)) {
+               fname = estrndup(fname, fname_len);
+               phar_unixify_path_separators(fname, fname_len);
+       }
 #endif
        basename = zend_memrchr(fname, '/', fname_len);
 
@@ -3616,6 +3637,10 @@ static void phar_add_file(phar_archive_data **pphar, char *filename, size_t file
        phar_entry_data *data;
        php_stream *contents_file = NULL;
        php_stream_statbuf ssb;
+#ifdef PHP_WIN32
+       char *save_filename;
+       ALLOCA_FLAG(filename_use_heap)
+#endif
 
        if (filename_len >= sizeof(".phar")-1) {
                start_pos = '/' == filename[0]; /* account for any leading slash: multiple-leads handled elsewhere */
@@ -3625,6 +3650,15 @@ static void phar_add_file(phar_archive_data **pphar, char *filename, size_t file
                }
        }
 
+#ifdef PHP_WIN32
+       save_filename = filename;
+       if (memchr(filename, '\\', filename_len)) {
+               filename = do_alloca(filename_len + 1, filename_use_heap);
+               memcpy(filename, save_filename, filename_len);
+               filename[filename_len] = '\0';
+       }
+#endif
+
        if (!(data = phar_get_or_create_entry_data((*pphar)->fname, (*pphar)->fname_len, filename, filename_len, "w+b", 0, &error, 1))) {
                if (error) {
                        zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s does not exist and cannot be created: %s", filename, error);
@@ -3632,7 +3666,7 @@ static void phar_add_file(phar_archive_data **pphar, char *filename, size_t file
                } else {
                        zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s does not exist and cannot be created", filename);
                }
-               return;
+               goto finish;
        } else {
                if (error) {
                        efree(error);
@@ -3643,12 +3677,12 @@ static void phar_add_file(phar_archive_data **pphar, char *filename, size_t file
                                contents_len = php_stream_write(data->fp, cont_str, cont_len);
                                if (contents_len != cont_len) {
                                        zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
-                                       return;
+                                       goto finish;
                                }
                        } else {
                                if (!(php_stream_from_zval_no_verify(contents_file, zresource))) {
                                        zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
-                                       return;
+                                       goto finish;
                                }
                                php_stream_copy_to_stream_ex(contents_file, data->fp, PHP_STREAM_COPY_ALL, &contents_len);
                        }
@@ -3678,6 +3712,14 @@ static void phar_add_file(phar_archive_data **pphar, char *filename, size_t file
                        efree(error);
                }
        }
+
+finish: ;
+#ifdef PHP_WIN32
+       if (filename != save_filename) {
+               free_alloca(filename, filename_use_heap);
+               filename = save_filename;
+       }
+#endif
 }
 /* }}} */