From de5aaaa74c319e4fd2cdcec0cfb57c0ae1edc14a Mon Sep 17 00:00:00 2001 From: Greg Beaver Date: Thu, 15 May 2008 16:09:01 +0000 Subject: [PATCH] fix potentially major security hole: modification/creation of files in .phar directory enabled in many locations which then allows easy creation of tar/zip-based phar archives with a simple rename even when phar.readonly=1. Plug the hole very tightly, allowing read access to files, and also excluding them from opendir() output --- ext/phar/dirstream.c | 16 +++- ext/phar/phar_internal.h | 8 +- ext/phar/phar_object.c | 93 ++++++++++++++++++--- ext/phar/stream.c | 7 +- ext/phar/tests/030.phpt | 20 +++-- ext/phar/tests/addfuncs.phpt | 12 +++ ext/phar/tests/mkdir.phpt | 6 ++ ext/phar/tests/mounteddir.phpt | 6 ++ ext/phar/tests/phar_buildfromiterator4.phpt | 21 ++++- ext/phar/tests/phar_copy.phpt | 12 +++ ext/phar/tests/phar_extract2.phpt | 20 ----- ext/phar/tests/phar_offset_check.phpt | 4 +- ext/phar/tests/phar_offset_get_error.phpt | 14 ++++ ext/phar/tests/tar/dir.phpt | 2 + ext/phar/tests/tar/tar_003.phpt | 4 - ext/phar/tests/zf_test.phpt | 1 - ext/phar/util.c | 27 ++++-- 17 files changed, 211 insertions(+), 62 deletions(-) diff --git a/ext/phar/dirstream.c b/ext/phar/dirstream.c index c1e8f31661..b411b5c1af 100644 --- a/ext/phar/dirstream.c +++ b/ext/phar/dirstream.c @@ -196,8 +196,9 @@ static php_stream *phar_make_dirstream(char *dir, HashTable *manifest TSRMLS_DC) ALLOC_HASHTABLE(data); zend_hash_init(data, 64, zend_get_hash_value, NULL, 0); - if (*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) { + if ((*dir == '/' && dirlen == 1 && (manifest->nNumOfElements == 0)) || (dirlen >= sizeof(".phar")-1 && !memcmp(dir, ".phar", sizeof(".phar")-1))) { /* make empty root directory for empty phar */ + /* make empty directory for .phar magic directory */ efree(dir); return php_stream_alloc(&phar_dir_ops, data, NULL, "r"); } @@ -217,6 +218,13 @@ static php_stream *phar_make_dirstream(char *dir, HashTable *manifest TSRMLS_DC) } if (*dir == '/') { /* root directory */ + if (keylen >= sizeof(".phar")-1 && !memcmp(str_key, ".phar", sizeof(".phar")-1)) { + /* do not add any magic entries to this directory */ + if (SUCCESS != zend_hash_move_forward(manifest)) { + break; + } + continue; + } if (NULL != (found = (char *) memchr(str_key, '/', keylen))) { /* the entry has a path separator and is a subdirectory */ entry = (char *) safe_emalloc(found - str_key, 1, 1); @@ -446,7 +454,7 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, char *url_from, int mode, in return FAILURE; } - if ((e = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path + 1), 2, &error TSRMLS_CC))) { + if ((e = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path + 1), 2, &error, 1 TSRMLS_CC))) { /* directory exists, or is a subdirectory of an existing file */ if (e->is_temp_dir) { efree(e->filename); @@ -462,7 +470,7 @@ int phar_wrapper_mkdir(php_stream_wrapper *wrapper, char *url_from, int mode, in php_url_free(resource); return FAILURE; } - if ((e = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path + 1), 0, &error TSRMLS_CC))) { + if ((e = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path + 1), 0, &error, 1 TSRMLS_CC))) { /* entry exists as a file */ php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: cannot create directory \"%s\" in phar \"%s\", file already exists", resource->path+1, resource->host); php_url_free(resource); @@ -565,7 +573,7 @@ int phar_wrapper_rmdir(php_stream_wrapper *wrapper, char *url, int options, php_ return FAILURE; } - if (!(entry = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path) - 1, 2, &error TSRMLS_CC))) { + if (!(entry = phar_get_entry_info_dir(phar, resource->path + 1, strlen(resource->path) - 1, 2, &error, 1 TSRMLS_CC))) { if (error) { php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: cannot remove directory \"%s\" in phar \"%s\", %s", resource->path+1, resource->host, error); efree(error); diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h index cec12ead31..51f885ea01 100755 --- a/ext/phar/phar_internal.h +++ b/ext/phar/phar_internal.h @@ -445,10 +445,10 @@ extern php_stream_wrapper php_stream_phar_wrapper; int phar_archive_delref(phar_archive_data *phar TSRMLS_DC); int phar_entry_delref(phar_entry_data *idata TSRMLS_DC); -phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, int path_len, char **error TSRMLS_DC); -phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, int path_len, char dir, char **error TSRMLS_DC); -phar_entry_data *phar_get_or_create_entry_data(char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error TSRMLS_DC); -int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error TSRMLS_DC); +phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, int path_len, char **error, int security TSRMLS_DC); +phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, int path_len, char dir, char **error, int security TSRMLS_DC); +phar_entry_data *phar_get_or_create_entry_data(char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error, int security TSRMLS_DC); +int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error, int security TSRMLS_DC); int phar_flush(phar_archive_data *archive, char *user_stub, long len, int convert, char **error TSRMLS_DC); int phar_detect_phar_fname_ext(const char *filename, int check_length, const char **ext_str, int *ext_len, int executable, int for_create, int is_complete TSRMLS_DC); int phar_split_fname(char *filename, int filename_len, char **arch, int *arch_len, char **entry, int *entry_len, int executable, int for_create TSRMLS_DC); diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 1e37e5d8c4..36bb09ef07 100755 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -327,7 +327,7 @@ static void phar_do_404(char *fname, int fname_len, char *f404, int f404_len, ch phar_entry_data *phar; char *error; if (f404_len) { - if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, f404, f404_len, "r", 0, &error TSRMLS_CC)) { + if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, f404, f404_len, "r", 0, &error, 1 TSRMLS_CC)) { if (error) { efree(error); } @@ -672,7 +672,7 @@ PHP_METHOD(Phar, webPhar) entry = estrndup("/index.php", sizeof("/index.php")); entry_len = sizeof("/index.php")-1; } - if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, entry, entry_len, "r", 0, NULL TSRMLS_CC)) { + if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, entry, entry_len, "r", 0, NULL, 1 TSRMLS_CC)) { phar_do_404(fname, fname_len, f404, f404_len, entry, entry_len TSRMLS_CC); if (free_pathinfo) { efree(path_info); @@ -711,7 +711,7 @@ PHP_METHOD(Phar, webPhar) } } - if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, entry, entry_len, "r", 0, &error TSRMLS_CC)) { + if (FAILURE == phar_get_entry_data(&phar, fname, fname_len, entry, entry_len, "r", 0, &error, 1 TSRMLS_CC)) { phar_do_404(fname, fname_len, f404, f404_len, entry, entry_len TSRMLS_CC); #ifdef PHP_WIN32 efree(fname); @@ -1503,12 +1503,31 @@ phar_spl_fileinfo: } after_open_fp: - if (!(data = phar_get_or_create_entry_data(phar_obj->arc.archive->fname, phar_obj->arc.archive->fname_len, str_key, str_key_len, "w+b", 0, &error TSRMLS_CC))) { + if (str_key_len >= sizeof(".phar")-1 && !memcmp(str_key, ".phar", sizeof(".phar")-1)) { + /* silently skip any files that would be added to the magic .phar directory */ + if (save) { + efree(save); + } + if (temp) { + efree(temp); + } + if (opened) { + efree(opened); + } + if (close_fp) { + php_stream_close(fp); + } + return ZEND_HASH_APPLY_KEEP; + } + if (!(data = phar_get_or_create_entry_data(phar_obj->arc.archive->fname, phar_obj->arc.archive->fname_len, str_key, str_key_len, "w+b", 0, &error, 1 TSRMLS_CC))) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Entry %s cannot be created: %s", str_key, error); efree(error); if (save) { efree(save); } + if (opened) { + efree(opened); + } if (temp) { efree(temp); } @@ -2992,6 +3011,20 @@ PHP_METHOD(Phar, copy) RETURN_FALSE; } + if (oldfile_len >= sizeof(".phar")-1 && !memcmp(oldfile, ".phar", sizeof(".phar")-1)) { + /* can't copy a meta file */ + zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, + "file \"%s\" cannot be copied to file \"%s\", cannot copy Phar meta-file in %s", oldfile, newfile, phar_obj->arc.archive->fname); + RETURN_FALSE; + } + + if (newfile_len >= sizeof(".phar")-1 && !memcmp(newfile, ".phar", sizeof(".phar")-1)) { + /* can't copy a meta file */ + zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, + "file \"%s\" cannot be copied to file \"%s\", cannot copy to Phar meta-file in %s", oldfile, newfile, phar_obj->arc.archive->fname); + RETURN_FALSE; + } + if (!zend_hash_exists(&phar_obj->arc.archive->manifest, oldfile, (uint) oldfile_len) || SUCCESS != zend_hash_find(&phar_obj->arc.archive->manifest, oldfile, (uint) oldfile_len, (void**)&oldentry) || oldentry->is_deleted) { zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "file \"%s\" cannot be copied to file \"%s\", file does not exist in %s", oldfile, newfile, phar_obj->arc.archive->fname); @@ -3077,6 +3110,11 @@ PHP_METHOD(Phar, offsetExists) RETURN_FALSE; } } + + if (fname_len >= sizeof(".phar")-1 && !memcmp(fname, ".phar", sizeof(".phar")-1)) { + /* none of these are real files, so they don't exist */ + RETURN_FALSE; + } RETURN_TRUE; } else { RETURN_FALSE; @@ -3098,10 +3136,26 @@ PHP_METHOD(Phar, offsetGet) if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &fname, &fname_len) == FAILURE) { return; } - - if (!(entry = phar_get_entry_info_dir(phar_obj->arc.archive, fname, fname_len, 1, &error TSRMLS_CC))) { + + /* security is 0 here so that we can get a better error message than "entry doesn't exist" */ + if (!(entry = phar_get_entry_info_dir(phar_obj->arc.archive, fname, fname_len, 1, &error, 0 TSRMLS_CC))) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Entry %s does not exist%s%s", fname, error?", ":"", error?error:""); } else { + if (fname_len == sizeof(".phar/stub.php")-1 && !memcmp(fname, ".phar/stub.php", sizeof(".phar/stub.php")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot get stub \".phar/stub.php\" directly in phar \"%s\", use getStub", phar_obj->arc.archive->fname); + return; + } + + if (fname_len == sizeof(".phar/alias.txt")-1 && !memcmp(fname, ".phar/alias.txt", sizeof(".phar/alias.txt")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot get alias \".phar/alias.txt\" directly in phar \"%s\", use getAlias", phar_obj->arc.archive->fname); + return; + } + + if (fname_len >= sizeof(".phar")-1 && !memcmp(fname, ".phar", sizeof(".phar")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot directly get any files or directories in magic \".phar\" directory", phar_obj->arc.archive->fname); + return; + } + if (entry->is_temp_dir) { efree(entry->filename); efree(entry); @@ -3125,7 +3179,12 @@ static void phar_add_file(phar_archive_data *phar, char *filename, int filename_ phar_entry_data *data; php_stream *contents_file; - if (!(data = phar_get_or_create_entry_data(phar->fname, phar->fname_len, filename, filename_len, "w+b", 0, &error TSRMLS_CC))) { + if (filename_len >= sizeof(".phar")-1 && !memcmp(filename, ".phar", sizeof(".phar")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot create any files in magic \".phar\" directory", phar->fname); + return; + } + + if (!(data = phar_get_or_create_entry_data(phar->fname, phar->fname_len, filename, filename_len, "w+b", 0, &error, 1 TSRMLS_CC))) { if (error) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Entry %s does not exist and cannot be created: %s", filename, error); efree(error); @@ -3170,7 +3229,7 @@ static void phar_mkdir(phar_archive_data *phar, char *dirname, int dirname_len T char *error; phar_entry_data *data; - if (!(data = phar_get_or_create_entry_data(phar->fname, phar->fname_len, dirname, dirname_len, "w+b", 2, &error TSRMLS_CC))) { + if (!(data = phar_get_or_create_entry_data(phar->fname, phar->fname_len, dirname, dirname_len, "w+b", 2, &error, 1 TSRMLS_CC))) { if (error) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Directory %s does not exist and cannot be created: %s", dirname, error); efree(error); @@ -3213,15 +3272,20 @@ PHP_METHOD(Phar, offsetSet) return; } - if ((phar_obj->arc.archive->is_tar || phar_obj->arc.archive->is_zip) && fname_len == sizeof(".phar/stub.php")-1 && !memcmp(fname, ".phar/stub.php", sizeof(".phar/stub.php")-1)) { + if (fname_len == sizeof(".phar/stub.php")-1 && !memcmp(fname, ".phar/stub.php", sizeof(".phar/stub.php")-1)) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot set stub \".phar/stub.php\" directly in phar \"%s\", use setStub", phar_obj->arc.archive->fname); return; } - if ((phar_obj->arc.archive->is_tar || phar_obj->arc.archive->is_zip) && fname_len == sizeof(".phar/alias.txt")-1 && !memcmp(fname, ".phar/alias.txt", sizeof(".phar/alias.txt")-1)) { + if (fname_len == sizeof(".phar/alias.txt")-1 && !memcmp(fname, ".phar/alias.txt", sizeof(".phar/alias.txt")-1)) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot set alias \".phar/alias.txt\" directly in phar \"%s\", use setAlias", phar_obj->arc.archive->fname); return; } + + if (fname_len >= sizeof(".phar")-1 && !memcmp(fname, ".phar", sizeof(".phar")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot set any files or directories in magic \".phar\" directory", phar_obj->arc.archive->fname); + return; + } phar_add_file(phar_obj->arc.archive, fname, fname_len, cont_str, cont_len, zresource TSRMLS_CC); } /* }}} */ @@ -3280,6 +3344,10 @@ PHP_METHOD(Phar, addEmptyDir) return; } + if (dirname_len >= sizeof(".phar")-1 && !memcmp(dirname, ".phar", sizeof(".phar")-1)) { + zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Cannot create a directory in magic \".phar\" directory"); + return; + } phar_mkdir(phar_obj->arc.archive, dirname, dirname_len TSRMLS_CC); } /* }}} */ @@ -3548,6 +3616,9 @@ static int phar_extract_file(zend_bool overwrite, phar_entry_info *entry, char * /* silently ignore mounted entries */ return SUCCESS; } + if (entry->filename_len >= sizeof(".phar")-1 && !memcmp(entry->filename, ".phar", sizeof(".phar")-1)) { + return SUCCESS; + } len = spprintf(&fullpath, 0, "%s/%s", dest, entry->filename); if (len >= MAXPATHLEN) { char *tmp; @@ -3834,7 +3905,7 @@ PHP_METHOD(PharFileInfo, __construct) return; } - if ((entry_info = phar_get_entry_info_dir(phar_data, entry, entry_len, 1, &error TSRMLS_CC)) == NULL) { + if ((entry_info = phar_get_entry_info_dir(phar_data, entry, entry_len, 1, &error, 1 TSRMLS_CC)) == NULL) { zend_throw_exception_ex(spl_ce_RuntimeException, 0 TSRMLS_CC, "Cannot access phar file entry '%s' in archive '%s'%s%s", entry, arch, error?", ":"", error?error:""); efree(arch); diff --git a/ext/phar/stream.c b/ext/phar/stream.c index e6c57a89ab..3bd2bd935d 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -178,7 +178,7 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, char *pat /* strip leading "/" */ internal_file = estrdup(resource->path + 1); if (mode[0] == 'w' || (mode[0] == 'r' && mode[1] == '+')) { - if (NULL == (idata = phar_get_or_create_entry_data(resource->host, host_len, internal_file, strlen(internal_file), mode, 0, &error TSRMLS_CC))) { + if (NULL == (idata = phar_get_or_create_entry_data(resource->host, host_len, internal_file, strlen(internal_file), mode, 0, &error, 1 TSRMLS_CC))) { if (error) { php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error); efree(error); @@ -223,7 +223,8 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, char *pat } return fpf; } else { - if ((FAILURE == phar_get_entry_data(&idata, resource->host, host_len, internal_file, strlen(internal_file), "r", 0, &error TSRMLS_CC)) || !idata) { + /* read-only access is allowed to magic files in .phar directory */ + if ((FAILURE == phar_get_entry_data(&idata, resource->host, host_len, internal_file, strlen(internal_file), "r", 0, &error, 0 TSRMLS_CC)) || !idata) { if (error) { php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error); efree(error); @@ -675,7 +676,7 @@ static int phar_wrapper_unlink(php_stream_wrapper *wrapper, char *url, int optio /* need to copy to strip leading "/", will get touched again */ internal_file = estrdup(resource->path + 1); - if (FAILURE == phar_get_entry_data(&idata, resource->host, strlen(resource->host), internal_file, strlen(internal_file), "r", 0, &error TSRMLS_CC)) { + if (FAILURE == phar_get_entry_data(&idata, resource->host, strlen(resource->host), internal_file, strlen(internal_file), "r", 0, &error, 1 TSRMLS_CC)) { /* constraints of fp refcount were not met */ if (error) { php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "unlink of \"%s\" failed: %s", url, error); diff --git a/ext/phar/tests/030.phpt b/ext/phar/tests/030.phpt index afaeb1a802..e1e94dba80 100755 --- a/ext/phar/tests/030.phpt +++ b/ext/phar/tests/030.phpt @@ -11,11 +11,12 @@ $pname = 'phar://' . $fname; $file = ''; $files = array(); -$files['a.php'] = ''; -$files['b.php'] = ''; -$files['b/c.php'] = ''; -$files['b/d.php'] = ''; -$files['e.php'] = ''; +$files['a.php'] = ''; +$files['b.php'] = ''; +$files['b/c.php'] = ''; +$files['b/d.php'] = ''; +$files['e.php'] = ''; +$files['.phar/test'] = ''; include 'files/phar_test.inc'; @@ -23,6 +24,13 @@ Phar::loadPhar($fname); require $pname . '/a.php'; +$p = new Phar($fname); +var_dump(isset($p['.phar/test'])); +try { +$p['.phar/test']; +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} ?> ===DONE=== --CLEAN-- @@ -35,4 +43,6 @@ This is b This is b/c This is b/d This is e +bool(false) +Cannot directly get any files or directories in magic ".phar" directory ===DONE=== diff --git a/ext/phar/tests/addfuncs.phpt b/ext/phar/tests/addfuncs.phpt index d9a183fd68..4312bff1a0 100644 --- a/ext/phar/tests/addfuncs.phpt +++ b/ext/phar/tests/addfuncs.phpt @@ -28,6 +28,16 @@ $phar->addFile(dirname(__FILE__) . '/does/not/exist'); } catch (Exception $e) { echo $e->getMessage() . "\n"; } +try { +$phar->addFile($pname . '/a', '.phar/stub.php'); +} catch (Exception $e) { +echo $e->getMessage() . "\n"; +} +try { +$phar->addFromString('.phar/stub.php', 'hi'); +} catch (Exception $e) { +echo $e->getMessage() . "\n"; +} ?> ===DONE=== --CLEAN-- @@ -38,4 +48,6 @@ hi Entry phar://%saddfuncs.phar.php/a does not exist and cannot be created: phar error: invalid path "phar://%saddfuncs.phar.php/a" contains double slash Entry a does not exist and cannot be created: phar error: file "a" in phar "%saddfuncs.phar.php" cannot be opened for writing, readable file pointers are open phar error: unable to open file "%s/does/not/exist" to add to phar archive +Cannot create any files in magic ".phar" directory +Cannot create any files in magic ".phar" directory ===DONE=== \ No newline at end of file diff --git a/ext/phar/tests/mkdir.phpt b/ext/phar/tests/mkdir.phpt index af8bceb32c..45c1d4674f 100644 --- a/ext/phar/tests/mkdir.phpt +++ b/ext/phar/tests/mkdir.phpt @@ -19,6 +19,11 @@ rmdir('phar://foo.phar'); rmdir($pname . '/a'); $a->addEmptyDir('bb'); $a->addEmptyDir('bb'); +try { +$a->addEmptyDir('.phar'); +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} ?> ===DONE=== --CLEAN-- @@ -38,4 +43,5 @@ Warning: rmdir(): phar error: cannot remove directory "phar://", no phar archive Warning: rmdir(): phar error: cannot remove directory "" in phar "foo.phar", directory does not exist in %smkdir.php on line %d Warning: rmdir(): phar error: cannot remove directory "a" in phar "%smkdir.phar.php", phar error: path "a" exists and is a not a directory in %smkdir.php on line %d +Cannot create a directory in magic ".phar" directory ===DONE=== \ No newline at end of file diff --git a/ext/phar/tests/mounteddir.phpt b/ext/phar/tests/mounteddir.phpt index c438eefb1d..51a42695d7 100644 --- a/ext/phar/tests/mounteddir.phpt +++ b/ext/phar/tests/mounteddir.phpt @@ -17,6 +17,11 @@ echo file_get_contents(Phar::running(1) . "/testit/directory"), "\n"; echo file_get_contents(Phar::running(1) . "/testit/existing.txt"), "\n"; include "testit/extfile.php"; include "testit/extfile2.php"; +try { +Phar::mount(".phar/stub.php", dirname(Phar::running(0)) . "/testit/extfile.php"); +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} ?>'; $a['testit/existing.txt'] = 'oops'; $a->setStub('buildFromIterator(new myIterator(array('a' => basename(__FILE__, 'php') . 'phpt')))); + var_dump($phar->buildFromIterator(new myIterator( + array( + 'a' => basename(__FILE__, 'php') . 'phpt', + // demonstrate that none of these are added + '.phar/stub.php' => basename(__FILE__, 'php') . 'phpt', + '.phar/alias.txt' => basename(__FILE__, 'php') . 'phpt', + '.phar/oops' => basename(__FILE__, 'php') . 'phpt', + )))); } catch (Exception $e) { var_dump(get_class($e)); echo $e->getMessage() . "\n"; @@ -57,6 +64,18 @@ current key next valid +current +key +next +valid +current +key +next +valid +current +key +next +valid array(1) { ["a"]=> string(%d) "%sphar_buildfromiterator4.phpt" diff --git a/ext/phar/tests/phar_copy.phpt b/ext/phar/tests/phar_copy.phpt index 0a43947bd1..382b7f0754 100644 --- a/ext/phar/tests/phar_copy.phpt +++ b/ext/phar/tests/phar_copy.phpt @@ -57,6 +57,16 @@ echo $e->getMessage() . "\n"; $p2['a']->compress(Phar::GZ); $p2->copy('a', 'd'); echo $p2['d']->getContent() . "\n"; +try { +$p2->copy('d', '.phar/stub.php'); +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} +try { +$p2->copy('.phar/stub.php', 'd'); +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} ?> ===DONE=== --CLEAN-- @@ -69,4 +79,6 @@ a: hib: hic: hia file "notexisting" cannot be copied to file "another", file does not exist in %sphar_copy2.phar.php file "a" cannot be copied to file "b", file must not already exist in phar %sphar_copy2.phar.php hi +file "d" cannot be copied to file ".phar/stub.php", cannot copy to Phar meta-file in %sphar_copy2.phar.php +file ".phar/stub.php" cannot be copied to file "d", cannot copy Phar meta-file in %sphar_copy2.phar.php ===DONE=== \ No newline at end of file diff --git a/ext/phar/tests/phar_extract2.phpt b/ext/phar/tests/phar_extract2.phpt index 9763cca476..64b8a74588 100644 --- a/ext/phar/tests/phar_extract2.phpt +++ b/ext/phar/tests/phar_extract2.phpt @@ -33,16 +33,6 @@ foreach ($extracted as $out) { echo "$out\n"; } -$variations = array('phar', '.phar', '/phar', '/.phar', '.phar/stub.php', '.phar/alias.txt', '/stub.php', '/alias.txt', 'stub.php', 'alias.txt'); - -foreach ($variations as $var) { - try { - $phar->extractTo(dirname(__FILE__) . '/extract1', $var); - } catch (Exception $e) { - echo $e->getMessage()."\n"; - } -} - ?> ===DONE=== --CLEAN-- @@ -67,14 +57,4 @@ $dir = dirname(__FILE__) . '/extract1/'; %sextract%csubdir %sextract%csubdir%cectory %sextract%csubdir%cectory%cfile.txt -Phar Error: attempted to extract non-existent file "phar" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file ".phar" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "/phar" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "/.phar" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file ".phar/stub.php" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file ".phar/alias.txt" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "/stub.php" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "/alias.txt" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "stub.php" from phar "%stempmanifest2.phar.php" -Phar Error: attempted to extract non-existent file "alias.txt" from phar "%stempmanifest2.phar.php" ===DONE=== diff --git a/ext/phar/tests/phar_offset_check.phpt b/ext/phar/tests/phar_offset_check.phpt index 279b359f4e..7e00d4b50e 100644 --- a/ext/phar/tests/phar_offset_check.phpt +++ b/ext/phar/tests/phar_offset_check.phpt @@ -66,11 +66,13 @@ var_dump($phar->getAlias()); ===DONE=== --CLEAN-- ---EXPECT-- +--EXPECTF-- Entry .phar/stub.php does not exist Entry .phar/alias.txt does not exist +Cannot set stub ".phar/stub.php" directly in phar "%sphar_offset_check.phar.php", use setStub int(6661) int(6661) +Cannot set alias ".phar/alias.txt" directly in phar "%sphar_offset_check.phar.php", use setAlias string(5) "susan" string(5) "susan" ===DONE=== diff --git a/ext/phar/tests/phar_offset_get_error.phpt b/ext/phar/tests/phar_offset_get_error.phpt index ad0223697d..dade4726d9 100755 --- a/ext/phar/tests/phar_offset_get_error.phpt +++ b/ext/phar/tests/phar_offset_get_error.phpt @@ -27,6 +27,18 @@ catch(Exception $e) } include($pname . $iname); + +// extra coverage +try { +$p['.phar/oops'] = 'hi'; +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} +try { +$a = $p['.phar/stub.php']; +} catch (Exception $e) { +echo $e->getMessage(),"\n"; +} ?> ===DONE=== --CLEAN-- @@ -34,4 +46,6 @@ include($pname . $iname); --EXPECT-- Entry /error/.. does not exist and cannot be created: phar error: invalid path "/error/.." contains upper directory reference foobar +Cannot set any files or directories in magic ".phar" directory +Entry .phar/stub.php does not exist ===DONE=== diff --git a/ext/phar/tests/tar/dir.phpt b/ext/phar/tests/tar/dir.phpt index 53bf65f9a7..64d5d12761 100644 --- a/ext/phar/tests/tar/dir.phpt +++ b/ext/phar/tests/tar/dir.phpt @@ -17,6 +17,7 @@ $phar = new Phar($fname); var_dump($phar->isFileFormat(Phar::TAR)); $phar->addEmptyDir('test'); +var_dump(isset($phar['.phar/stub.php'])); var_dump($phar['test']->isDir()); var_dump($phar['test/']->isDir()); copy($fname, $fname2); @@ -35,6 +36,7 @@ var_dump(file_exists($pname3 . '/another/dir/')); --EXPECT-- bool(true) +bool(false) bool(true) bool(true) bool(true) diff --git a/ext/phar/tests/tar/tar_003.phpt b/ext/phar/tests/tar/tar_003.phpt index 47f1d6ca9b..b390805dc3 100644 --- a/ext/phar/tests/tar/tar_003.phpt +++ b/ext/phar/tests/tar/tar_003.phpt @@ -55,8 +55,6 @@ closedir($tar); --EXPECT-- hi there! dir -.phar -dir dir dir internal @@ -64,8 +62,6 @@ file tar_003.phpt second round dir -.phar -dir dir dir internal diff --git a/ext/phar/tests/zf_test.phpt b/ext/phar/tests/zf_test.phpt index 72796b0ee3..0e0da1fcc9 100644 --- a/ext/phar/tests/zf_test.phpt +++ b/ext/phar/tests/zf_test.phpt @@ -40,7 +40,6 @@ unlink(dirname(__FILE__) . '/zfapp.tgz'); unlink(dirname(__FILE__) . '/zfapp.phar.tar.gz'); ?> --EXPECTF-- -phar://%szfapp.phar.tar.gz/.phar/stub.php phar://%szfapp.phar.tar.gz/application/default/controllers/ErrorController.php phar://%szfapp.phar.tar.gz/application/default/controllers/IndexController.php phar://%szfapp.phar.tar.gz/application/default/views/scripts/error/error.phtml diff --git a/ext/phar/util.c b/ext/phar/util.c index 93587d0678..bc6ccaaa78 100644 --- a/ext/phar/util.c +++ b/ext/phar/util.c @@ -146,6 +146,11 @@ int phar_mount_entry(phar_archive_data *phar, char *filename, int filename_len, return FAILURE; } + if (path_len >= sizeof(".phar")-1 && !memcmp(path, ".phar", sizeof(".phar")-1)) { + /* no creating magic phar files by mounting them */ + return FAILURE; + } + is_phar = (filename_len > 7 && !memcmp(filename, "phar://", 7)); entry.phar = phar; @@ -483,7 +488,7 @@ not_stream: * appended, truncated, or read. For read, if the entry is marked unmodified, it is * assumed that the file pointer, if present, is opened for reading */ -int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error TSRMLS_DC) /* {{{ */ +int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error, int security TSRMLS_DC) /* {{{ */ { phar_archive_data *phar; phar_entry_info *entry; @@ -515,14 +520,14 @@ int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char return FAILURE; } if (allow_dir) { - if ((entry = phar_get_entry_info_dir(phar, path, path_len, allow_dir, for_create && !PHAR_G(readonly) && !phar->is_data ? NULL : error TSRMLS_CC)) == NULL) { + if ((entry = phar_get_entry_info_dir(phar, path, path_len, allow_dir, for_create && !PHAR_G(readonly) && !phar->is_data ? NULL : error, security TSRMLS_CC)) == NULL) { if (for_create && (!PHAR_G(readonly) || phar->is_data)) { return SUCCESS; } return FAILURE; } } else { - if ((entry = phar_get_entry_info(phar, path, path_len, for_create && !PHAR_G(readonly) && !phar->is_data ? NULL : error TSRMLS_CC)) == NULL) { + if ((entry = phar_get_entry_info(phar, path, path_len, for_create && !PHAR_G(readonly) && !phar->is_data ? NULL : error, security TSRMLS_CC)) == NULL) { if (for_create && (!PHAR_G(readonly) || phar->is_data)) { return SUCCESS; } @@ -608,7 +613,7 @@ int phar_get_entry_data(phar_entry_data **ret, char *fname, int fname_len, char /** * Create a new dummy file slot within a writeable phar for a newly created file */ -phar_entry_data *phar_get_or_create_entry_data(char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error TSRMLS_DC) /* {{{ */ +phar_entry_data *phar_get_or_create_entry_data(char *fname, int fname_len, char *path, int path_len, char *mode, char allow_dir, char **error, int security TSRMLS_DC) /* {{{ */ { phar_archive_data *phar; phar_entry_info *entry, etemp; @@ -626,7 +631,7 @@ phar_entry_data *phar_get_or_create_entry_data(char *fname, int fname_len, char return NULL; } - if (FAILURE == phar_get_entry_data(&ret, fname, fname_len, path, path_len, mode, allow_dir, error TSRMLS_CC)) { + if (FAILURE == phar_get_entry_data(&ret, fname, fname_len, path, path_len, mode, allow_dir, error, security TSRMLS_CC)) { return NULL; } else if (ret) { return ret; @@ -1118,9 +1123,9 @@ char * phar_decompress_filter(phar_entry_info * entry, int return_unknown) /* {{ /** * retrieve information on a file contained within a phar, or null if it ain't there */ -phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, int path_len, char **error TSRMLS_DC) /* {{{ */ +phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, int path_len, char **error, int security TSRMLS_DC) /* {{{ */ { - return phar_get_entry_info_dir(phar, path, path_len, 0, error TSRMLS_CC); + return phar_get_entry_info_dir(phar, path, path_len, 0, error, security TSRMLS_CC); } /* }}} */ /** @@ -1128,7 +1133,7 @@ phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, int pa * allow_dir is 0 for none, 1 for both empty directories in the phar and temp directories, and 2 for only * valid pre-existing empty directory entries */ -phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, int path_len, char dir, char **error TSRMLS_DC) /* {{{ */ +phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, int path_len, char dir, char **error, int security TSRMLS_DC) /* {{{ */ { const char *pcr_error; phar_entry_info *entry; @@ -1144,6 +1149,12 @@ phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, in *error = NULL; } + if (security && path_len >= sizeof(".phar")-1 && !memcmp(path, ".phar", sizeof(".phar")-1)) { + if (error) { + spprintf(error, 4096, "phar error: cannot directly access magic \".phar\" directory or files within it"); + } + return NULL; + } if (!path_len && !dir) { if (error) { spprintf(error, 4096, "phar error: invalid path \"%s\" must not be empty", path); -- 2.40.0