Resolve open_basedir paths on ini update

Author: iluuu1994

Zend/tests/gh10469.phpt

@@ -8,9 +8,9 @@ $originalDir = __DIR__;
 $tmpDir = $originalDir . '/gh10469_tmp';
 @mkdir($tmpDir, 0777, true);
 chdir($tmpDir);
-ini_set('open_basedir', ini_get('open_basedir') . ':./..');
-ini_set('open_basedir', ini_get('open_basedir') . ':./../');
-ini_set('open_basedir', ini_get('open_basedir') . ':/a/');
+ini_set('open_basedir', ini_get('open_basedir') . ':.' . DIRECTORY_SEPARATOR . '..');
+ini_set('open_basedir', ini_get('open_basedir') . ':.' . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR);
+ini_set('open_basedir', ini_get('open_basedir') . ':' . DIRECTORY_SEPARATOR . 'a' . DIRECTORY_SEPARATOR);
 
 chdir($originalDir);
 var_dump(ini_get('open_basedir'));
@@ -20,4 +20,4 @@ var_dump(ini_get('open_basedir'));
 @rmdir(__DIR__ . '/gh10469_tmp');
 ?>
 --EXPECTF--
-string(%d) "%stests"
+string(%d) "%stests:.%e..:.%e..%e"

Zend/zend.c

@@ -38,6 +38,11 @@
 #include "zend_call_stack.h"
 #include "zend_max_execution_timer.h"
 #include "Optimizer/zend_optimizer.h"
+#include "php.h"
+#include "php_globals.h"
+
+// FIXME: Breaks the declaration of the function below
+#undef zenderror
 
 static size_t global_map_ptr_last = 0;
 static bool startup_done = false;
@@ -1259,11 +1264,29 @@ void zend_call_destructors(void) /* {{{ */
 }
 /* }}} */
 
+static void zend_release_open_basedir(void)
+{
+	/* Release custom open_basedir config, this needs to happen before ini shutdown */
+	if (PG(open_basedir)) {
+		zend_ini_entry *ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "open_basedir", strlen("open_basedir"));
+		/* ini_entry->modified is unreliable, it might also be set when on_update has failed. */
+		if (ini_entry
+		 && ini_entry->modified
+		 && ini_entry->value != ini_entry->orig_value) {
+			efree(PG(open_basedir));
+			PG(open_basedir) = NULL;
+		}
+	}
+}
+
 ZEND_API void zend_deactivate(void) /* {{{ */
 {
 	/* we're no longer executing anything */
 	EG(current_execute_data) = NULL;
 
+	/* Needs to run before zend_ini_deactivate(). */
+	zend_release_open_basedir();
+
 	zend_try {
 		shutdown_scanner();
 	} zend_end_try();

ext/session/tests/session_save_path_variation5.phpt

@@ -28,6 +28,7 @@ if (file_exists($sessions) === TRUE) {
 
 var_dump(mkdir($sessions));
 var_dump(chdir($sessions));
+ini_set('open_basedir', '.');
 ini_set("session.save_path", $directory);
 var_dump(session_save_path());
 
@@ -45,6 +46,6 @@ rmdir($sessions);
 bool(true)
 bool(true)
 
-Warning: ini_set(): open_basedir restriction in effect. File(%s) is not within the allowed path(s): (.) in %s on line %d
+Warning: ini_set(): open_basedir restriction in effect. File(%s) is not within the allowed path(s): (%ssession_save_path_variation5) in %s on line %d
 string(0) ""
 Done

main/fopen_wrappers.c

@@ -38,6 +38,7 @@
 #include "ext/standard/php_standard.h"
 #include "zend_compile.h"
 #include "php_network.h"
+#include "zend_smart_str.h"
 
 #if HAVE_PWD_H
 #include <pwd.h>
@@ -81,60 +82,50 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
 		return SUCCESS;
 	}
 
-	/* Otherwise we're in runtime */
-	if (!*p || !**p) {
-		/* open_basedir not set yet, go ahead and give it a value */
-		*p = ZSTR_VAL(new_value);
-		return SUCCESS;
-	}
-
 	/* Shortcut: When we have a open_basedir and someone tries to unset, we know it'll fail */
 	if (!new_value || !*ZSTR_VAL(new_value)) {
 		return FAILURE;
 	}
 
 	/* Is the proposed open_basedir at least as restrictive as the current setting? */
+	smart_str buf = {0};
 	ptr = pathbuf = estrdup(ZSTR_VAL(new_value));
 	while (ptr && *ptr) {
 		end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
 		if (end != NULL) {
 			*end = '\0';
 			end++;
 		}
-		/* Don't allow paths with a parent dir component (..) to be set at runtime */
-		char *substr_pos = ptr;
-		while (*substr_pos) {
-			// Check if we have a .. path component
-			if (substr_pos[0] == '.'
-			 && substr_pos[1] == '.'
-			 && (substr_pos[2] == '\0' || IS_SLASH(substr_pos[2]))) {
-				efree(pathbuf);
-				return FAILURE;
-			}
-			// Skip to the next path component
-			while (true) {
-				substr_pos++;
-				if (*substr_pos == '\0' || *substr_pos == DEFAULT_DIR_SEPARATOR) {
-					goto no_parent_dir_component;
-				} else if (IS_SLASH(*substr_pos)) {
-					// Also skip the slash
-					substr_pos++;
-					break;
-				}
-			}
+		char resolved_name[MAXPATHLEN + 1];
+		if (expand_filepath(ptr, resolved_name) == NULL) {
+			smart_str_free(&buf);
+			return FAILURE;
 		}
-no_parent_dir_component:
-		if (php_check_open_basedir_ex(ptr, 0) != 0) {
+		if (php_check_open_basedir_ex(resolved_name, 0) != 0) {
 			/* At least one portion of this open_basedir is less restrictive than the prior one, FAIL */
 			efree(pathbuf);
+			smart_str_free(&buf);
 			return FAILURE;
 		}
+		if (smart_str_get_len(&buf) != 0) {
+			smart_str_appendc(&buf, DEFAULT_DIR_SEPARATOR);
+		}
+		smart_str_appends(&buf, resolved_name);
 		ptr = end;
 	}
 	efree(pathbuf);
 
 	/* Everything checks out, set it */
-	*p = ZSTR_VAL(new_value);
+	if (*p) {
+		/* Unfortunately entry->modified has already been set to true so we compare entry->value
+		 * against entry->original_value. */
+		if (entry->value != entry->orig_value) {
+			efree(*p);
+		}
+	}
+	zend_string *tmp = smart_str_extract(&buf);
+	*p = estrdup(ZSTR_VAL(tmp));
+	zend_string_release(tmp);
 
 	return SUCCESS;
 }

main/php.h

@@ -296,7 +296,7 @@ ssize_t pread(int, void *, size_t, off64_t);
 #endif
 
 BEGIN_EXTERN_C()
-void phperror(char *error);
+void phperror(const char *error);
 PHPAPI size_t php_write(void *buf, size_t size);
 PHPAPI size_t php_printf(const char *format, ...) PHP_ATTRIBUTE_FORMAT(printf, 1, 2);
 PHPAPI size_t php_printf_unchecked(const char *format, ...);

tests/security/bug76359.phpt

@@ -10,7 +10,7 @@ chdir("..");
 chdir("..");
 ?>
 --EXPECTF--
-bool(false)
+string(%d) "%ssecurity"
 
 Warning: chdir(): open_basedir restriction in effect. File(..) is not within the allowed path(s): (%s) in %s on line %d
 --CLEAN--

tests/security/open_basedir_readlink.phpt

@@ -21,6 +21,7 @@ $symlink = ($initdir."/test/ok/symlink.txt");
 var_dump(symlink($target, $symlink));
 
 chdir($initdir."/test/ok");
+ini_set("open_basedir", ".");
 
 var_dump(readlink("symlink.txt"));
 var_dump(readlink("../ok/symlink.txt"));
@@ -49,24 +50,24 @@ bool(true)
 bool(true)
 bool(true)
 
-Warning: readlink(): open_basedir restriction in effect. File(symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: readlink(): open_basedir restriction in effect. File(../ok/symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(../ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: readlink(): open_basedir restriction in effect. File(../ok/./symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(../ok/./symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: readlink(): open_basedir restriction in effect. File(./symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(./symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: symlink(): open_basedir restriction in effect. File(%s/test/bad/bad.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: symlink(): open_basedir restriction in effect. File(%s/test/bad/bad.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 
-Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (.) in %s on line %d
+Warning: readlink(): open_basedir restriction in effect. File(%s/test/ok/symlink.txt) is not within the allowed path(s): (%sok) in %s on line %d
 bool(false)
 *** Finished testing open_basedir configuration [readlink] ***