From 1a9e6895f1d203f38655b52d5b6b823be7d14cbd Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 24 Jul 2022 15:13:29 +0100 Subject: [PATCH] Fix #65069: GlobIterator incorrect handling of open_basedir check This PR changes the glob stream wrapper so it impacts "glob://" streamsas well. The idea is to do a check for each found path instead of the pattern which was not working correctly. --- NEWS | 2 + UPGRADING | 3 ++ ext/spl/tests/bug65069.phpt | 57 ++++++++++++++++++++ ext/standard/tests/streams/glob-wrapper.phpt | 7 +-- main/streams/glob_wrapper.c | 45 ++++++++++++---- 5 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 ext/spl/tests/bug65069.phpt diff --git a/NEWS b/NEWS index ed1829bc2589c..e4f488e8aceb1 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,8 @@ PHP NEWS - SPL: . Fixed bug #69181 (READ_CSV|DROP_NEW_LINE drops newlines within fields). (cmb) + . Fixed bug #65069 (GlobIterator incorrect handling of open_basedir check). + (Jakub Zelenka) 21 Jul 2022, PHP 8.2.0beta1 diff --git a/UPGRADING b/UPGRADING index df3d92cdcc25f..75f65abdd3665 100644 --- a/UPGRADING +++ b/UPGRADING @@ -61,6 +61,9 @@ PHP 8.2 UPGRADE NOTES previously it was bool . SplFileObject::getChildren() now has a tentative return type of null, previously it was ?RecursiveIterator + . GlogIterator returns empty array if all paths are restricted by + open_basedir. Previously the error was returned but that behavior was not + consistent and did not work correctly. ======================================== 2. New Features diff --git a/ext/spl/tests/bug65069.phpt b/ext/spl/tests/bug65069.phpt new file mode 100644 index 0000000000000..384e3246c8840 --- /dev/null +++ b/ext/spl/tests/bug65069.phpt @@ -0,0 +1,57 @@ +--TEST-- +Bug #65069: GlobIterator fails to access files inside an open_basedir restricted dir +--FILE-- +getFilename() . "\n"; +} + +$spl_glob_it = new \GlobIterator(dirname(dirname($dirname)) . "/*/*/*"); +foreach ($spl_glob_it as $file_info) { + echo $file_info->getFilename() . "\n"; +} + +$spl_glob_empty = new \GlobIterator("$dirname/*.php"); +var_dump($spl_glob_empty->count()); + +// top directory +var_dump(iterator_to_array(new \GlobIterator(dirname(dirname($dirname))))); + +// not existing file +var_dump(iterator_to_array(new \GlobIterator("$file_path/bug65069-this-will-never-exists"))); + + +?> +--CLEAN-- + +--EXPECT-- +wonder.txt +file.text +wonder.txt +wonder12345 +int(0) +array(0) { +} +array(0) { +} diff --git a/ext/standard/tests/streams/glob-wrapper.phpt b/ext/standard/tests/streams/glob-wrapper.phpt index fcd7add1954fd..212a33f1440c5 100644 --- a/ext/standard/tests/streams/glob-wrapper.phpt +++ b/ext/standard/tests/streams/glob-wrapper.phpt @@ -5,6 +5,7 @@ open_basedir=/does_not_exist --SKIPIF-- --FILE-- open_basedir_used ? (int) pglob->open_basedir_indexmap_size : pglob->glob.gl_pathc; +} + PHPAPI int _php_glob_stream_get_count(php_stream *stream, int *pflags STREAMS_DC) /* {{{ */ { glob_s_t *pglob = (glob_s_t *)stream->abstract; @@ -87,7 +95,7 @@ PHPAPI int _php_glob_stream_get_count(php_stream *stream, int *pflags STREAMS_DC if (pflags) { *pflags = pglob->flags; } - return pglob->glob.gl_pathc; + return php_glob_stream_get_result_count(pglob); } else { if (pflags) { *pflags = 0; @@ -130,15 +138,21 @@ static ssize_t php_glob_stream_read(php_stream *stream, char *buf, size_t count) glob_s_t *pglob = (glob_s_t *)stream->abstract; php_stream_dirent *ent = (php_stream_dirent*)buf; const char *path; + int glob_result_count; + size_t index; /* avoid problems if someone mis-uses the stream */ if (count == sizeof(php_stream_dirent) && pglob) { - if (pglob->index < (size_t)pglob->glob.gl_pathc) { - php_glob_stream_path_split(pglob, pglob->glob.gl_pathv[pglob->index++], pglob->flags & GLOB_APPEND, &path); + glob_result_count = php_glob_stream_get_result_count(pglob); + if (pglob->index < (size_t) glob_result_count) { + index = pglob->open_basedir_used && pglob->open_basedir_indexmap ? + pglob->open_basedir_indexmap[pglob->index] : pglob->index; + php_glob_stream_path_split(pglob, pglob->glob.gl_pathv[index], pglob->flags & GLOB_APPEND, &path); + ++pglob->index; PHP_STRLCPY(ent->d_name, path, sizeof(ent->d_name), strlen(path)); return sizeof(php_stream_dirent); } - pglob->index = pglob->glob.gl_pathc; + pglob->index = glob_result_count; if (pglob->path) { efree(pglob->path); pglob->path = NULL; @@ -162,6 +176,9 @@ static int php_glob_stream_close(php_stream *stream, int close_handle) /* {{{ * if (pglob->pattern) { efree(pglob->pattern); } + if (pglob->open_basedir_indexmap) { + efree(pglob->open_basedir_indexmap); + } } efree(stream->abstract); return 0; @@ -198,7 +215,7 @@ static php_stream *php_glob_stream_opener(php_stream_wrapper *wrapper, const cha int options, zend_string **opened_path, php_stream_context *context STREAMS_DC) { glob_s_t *pglob; - int ret; + int ret, i; const char *tmp, *pos; if (!strncmp(path, "glob://", sizeof("glob://")-1)) { @@ -208,10 +225,6 @@ static php_stream *php_glob_stream_opener(php_stream_wrapper *wrapper, const cha } } - if (((options & STREAM_DISABLE_OPEN_BASEDIR) == 0) && php_check_open_basedir(path)) { - return NULL; - } - pglob = ecalloc(sizeof(*pglob), 1); if (0 != (ret = glob(path, pglob->flags & GLOB_FLAGMASK, NULL, &pglob->glob))) { @@ -224,6 +237,20 @@ static php_stream *php_glob_stream_opener(php_stream_wrapper *wrapper, const cha } } + /* if open_basedir in use, check and filter restricted paths */ + if ((options & STREAM_DISABLE_OPEN_BASEDIR) == 0) { + pglob->open_basedir_used = true; + for (i = 0; i < pglob->glob.gl_pathc; i++) { + if (!php_check_open_basedir_ex(pglob->glob.gl_pathv[i], 0)) { + if (!pglob->open_basedir_indexmap) { + pglob->open_basedir_indexmap = (size_t *) safe_emalloc( + pglob->glob.gl_pathc, sizeof(size_t), 0); + } + pglob->open_basedir_indexmap[pglob->open_basedir_indexmap_size++] = i; + } + } + } + pos = path; if ((tmp = strrchr(pos, '/')) != NULL) { pos = tmp+1;