1.
glob() function can cause crash in ZTS mode.
For relative path in ZTS we can find code:
[...]
cwd_skip = strlen(cwd)+1;
snprintf(work_pattern, MAXPATHLEN, "%s%c%s", cwd, DEFAULT_SLASH, pattern);
[...]
add_next_index_string(return_value, globbuf.gl_pathv[n]+cwd_skip, 1);
[...]
If pattern contains something like "./../../../../../../../*" ,
globbuf.gl_pathv[n] could me shorter than cwd_skip.
In result we can read from illegal area of memory.
2.
in code we can read:
/* we assume that any glob pattern will match files from one
directory only
so checking the dirname of the first match should be
sufficient */
But it's not true. Glob() can match files from more than one directory.
i.e: "./*/*" will match files from many directories
About my solution:
- in safe_mode / open_basedir set -- I dont allow use regular expresions
before last slash in pattern -- so glob will match files only from one
directory
- pattern is expanded using virtual_file_ex (without using realname of
course) to eliminate '..' '.' dirs. If pattern 'leaves' current
directory, will be used as absolute pattern (result will contain full
names - not relative ones).
- added missing globfree() calls.
- added (missing?) terminating '\0' in line 453
This patch is not so beautiful (my english too -- I'm not native english
speaker) , but I hope it will help you fix these bugs.
Best regards.
--
Marcin Obara
Index: ext/standard/dir.c
===================================================================
RCS file: /repository/php-src/ext/standard/dir.c,v
retrieving revision 1.109.2.18.2.1
diff -u -r1.109.2.18.2.1 dir.c
--- ext/standard/dir.c 12 Jun 2005 01:15:43 -0000 1.109.2.18.2.1
+++ ext/standard/dir.c 5 Dec 2005 07:51:59 -0000
@@ -351,10 +351,8 @@
{
char cwd[MAXPATHLEN];
int cwd_skip = 0;
-#ifdef ZTS
char work_pattern[MAXPATHLEN];
char *result;
-#endif
char *pattern = NULL;
int pattern_len;
long flags = 0;
@@ -364,23 +362,65 @@
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|l", &pattern, &pattern_len, &flags) == FAILURE)
return;
-#ifdef ZTS
+ if ((PG(safe_mode) || (PG(open_basedir) && *PG(open_basedir)))
+ && (pattern_len > 0)
+ && ((result = strrchr(pattern+1, DEFAULT_SLASH)) != NULL)
+ ) {
+ /* we have directories in pattern
+ * so we have to check if pattern can match many directories */
+ *result = '\0';
+ if (strchr(pattern, '?') || strchr(pattern, '*') || strchr(pattern, '[')) {
+ /* directory part of pattern contains wildcards - unsafe */
+ *result = DEFAULT_SLASH;
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, "safe_mode/open_basedir restriction in efect. Glob pattern can match files from only ONE directory.");
+ RETURN_FALSE;
+ }
+ *result = DEFAULT_SLASH;
+ }
+
if (!IS_ABSOLUTE_PATH(pattern, pattern_len)) {
+ cwd_state new_state;
+
result = VCWD_GETCWD(cwd, MAXPATHLEN);
if (!result) {
- cwd[0] = '\0';
- }
+ /* virtual_file_ex() with use_realpath == 0
+ * must have cwd_length > 0 to do anything
+ * if path is not absolute;
+ *
+ * maybe we should RETURN_FALSE here ?
+ * but I tried to be compatible with previous implementation
+ */
+ cwd[0] = DEFAULT_SLASH;
+ cwd[1] = '\0';
+ cwd_skip = 1;
+ } else {
+ cwd_skip = strlen(cwd)+1;
#ifdef PHP_WIN32
- if (IS_SLASH(*pattern)) {
- cwd[2] = '\0';
- }
+ if ((IS_SLASH(*pattern)) && (cwd_skip > 2)) {
+ cwd_skip = 2;
+ }
#endif
- cwd_skip = strlen(cwd)+1;
-
- snprintf(work_pattern, MAXPATHLEN, "%s%c%s", cwd, DEFAULT_SLASH, pattern);
+ }
+ new_state.cwd = strdup(cwd);
+ new_state.cwd_length = strlen(cwd);
+ if (virtual_file_ex(&new_state, pattern, NULL, 0)) {
+ free(new_state.cwd);
+ RETURN_FALSE;
+ }
+ ret = strlen(cwd);
+ n = (new_state.cwd_length > MAXPATHLEN - 1) ? (MAXPATHLEN - 1) : new_state.cwd_length;
+ if ((n < ret) || (strncmp(new_state.cwd, cwd, ret) != 0)) {
+ /* we MUST NOT use cwd_skip if:
+ * - pattern is shorter
+ * - pattern matches not current path
+ */
+ cwd_skip = 0;
+ }
+ memcpy(work_pattern, new_state.cwd, n);
+ work_pattern[n] = '\0';
+ free(new_state.cwd);
pattern = work_pattern;
}
-#endif
globbuf.gl_offs = 0;
if (0 != (ret = glob(pattern, flags & GLOB_FLAGMASK, NULL, &globbuf))) {
@@ -405,11 +445,18 @@
/* we assume that any glob pattern will match files from one directory only
so checking the dirname of the first match should be sufficient */
+ /* NO, glob can match files from more than one directory
+ * but we already checked pattern at the beginning of this function
+ * if safe_mode or open_basedir is set
+ */
strncpy(cwd, globbuf.gl_pathv[0], MAXPATHLEN);
+ cwd[MAXPATHLEN - 1] = '\0';
if (PG(safe_mode) && (!php_checkuid(cwd, NULL, CHECKUID_CHECK_FILE_AND_DIR))) {
+ globfree(&globbuf);
RETURN_FALSE;
}
if (php_check_open_basedir(cwd TSRMLS_CC)) {
+ globfree(&globbuf);
RETURN_FALSE;
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
