Skip to content

Commit

Permalink
Fix #65069: GlobIterator incorrect handling of open_basedir check
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bukka committed Jul 28, 2022
1 parent 520bb2e commit 1a9e689
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 14 deletions.
2 changes: 2 additions & 0 deletions NEWS
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions UPGRADING
Expand Up @@ -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
Expand Down
57 changes: 57 additions & 0 deletions ext/spl/tests/bug65069.phpt
@@ -0,0 +1,57 @@
--TEST--
Bug #65069: GlobIterator fails to access files inside an open_basedir restricted dir
--FILE--
<?php
$file_path = __DIR__;
// temp dirname used here
$dirname = "$file_path/bug65069";
// temp dir created
mkdir($dirname);

ini_set('open_basedir', $dirname);

// temp files created
touch("$dirname/wonder12345");
touch("$dirname/wonder.txt");
touch("$dirname/file.text");

$spl_glob_it = new \GlobIterator("$dirname/*.txt");
foreach ($spl_glob_it as $file_info) {
echo $file_info->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--
<?php
$file_path = dirname(__FILE__);
$dirname = "$file_path/bug65069";
unlink("$dirname/wonder12345");
unlink("$dirname/wonder.txt");
unlink("$dirname/file.text");
rmdir($dirname);
?>
--EXPECT--
wonder.txt
file.text
wonder.txt
wonder12345
int(0)
array(0) {
}
array(0) {
}
7 changes: 2 additions & 5 deletions ext/standard/tests/streams/glob-wrapper.phpt
Expand Up @@ -5,6 +5,7 @@ open_basedir=/does_not_exist
--SKIPIF--
<?php
if (!in_array("glob", stream_get_wrappers())) echo "skip";
?>
--FILE--
<?php

Expand All @@ -29,8 +30,4 @@ Warning: opendir(): open_basedir restriction in effect. File(%s) is not within t
Warning: opendir(%s): Failed to open directory: Operation not permitted in %s%eglob-wrapper.php on line 5
Failed to open %s
** Opening glob://%s

Warning: opendir(): open_basedir restriction in effect. File(%s) is not within the allowed path(s): (/does_not_exist) in %s%eglob-wrapper.php on line 5

Warning: opendir(glob://%s): Failed to open directory: operation failed in %s%eglob-wrapper.php on line 5
Failed to open glob://%s
No files in glob://%s
45 changes: 36 additions & 9 deletions main/streams/glob_wrapper.c
Expand Up @@ -41,6 +41,9 @@ typedef struct {
size_t path_len;
char *pattern;
size_t pattern_len;
size_t *open_basedir_indexmap;
size_t open_basedir_indexmap_size;
bool open_basedir_used;
} glob_s_t;

PHPAPI char* _php_glob_stream_get_path(php_stream *stream, size_t *plen STREAMS_DC) /* {{{ */
Expand Down Expand Up @@ -79,6 +82,11 @@ PHPAPI char* _php_glob_stream_get_pattern(php_stream *stream, size_t *plen STREA
}
/* }}} */

static inline int php_glob_stream_get_result_count(glob_s_t *pglob)
{
return pglob->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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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))) {
Expand All @@ -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;
Expand Down

0 comments on commit 1a9e689

Please sign in to comment.