From 127ad70782f9bf5c245a7b55a1c420f2d67b993f Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 24 Jul 2023 23:23:32 +0200 Subject: [PATCH] Fix open_basedir leak Fixes oss-fuzz #60741 Closes GH-11780 --- NEWS | 3 +++ Zend/tests/oss_fuzz_60741.phpt | 9 +++++++++ Zend/zend.c | 18 ------------------ main/fopen_wrappers.c | 18 ++++++++++-------- main/php_globals.h | 1 + 5 files changed, 23 insertions(+), 26 deletions(-) create mode 100644 Zend/tests/oss_fuzz_60741.phpt diff --git a/NEWS b/NEWS index 80e71e8f25d..072a28ae6c3 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.0beta2 +- Core: + . Fixed oss-fuzz #60741 (Leak in open_basedir). (ilutov) + - FFI: . Fix leaking definitions when using FFI::cdef()->new(...). (ilutov) diff --git a/Zend/tests/oss_fuzz_60741.phpt b/Zend/tests/oss_fuzz_60741.phpt new file mode 100644 index 00000000000..8064bf54592 --- /dev/null +++ b/Zend/tests/oss_fuzz_60741.phpt @@ -0,0 +1,9 @@ +--TEST-- +oss-fuzz #60741: Leak in open_basedir +--INI-- +open_basedir="{TMP}" +--FILE-- + +--EXPECT-- diff --git a/Zend/zend.c b/Zend/zend.c index 48a7bef867b..56770ac3111 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1271,29 +1271,11 @@ 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(); diff --git a/main/fopen_wrappers.c b/main/fopen_wrappers.c index bcc7f6740cc..d7644dcd06c 100644 --- a/main/fopen_wrappers.c +++ b/main/fopen_wrappers.c @@ -77,8 +77,12 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir) char *pathbuf, *ptr, *end; if (stage == PHP_INI_STAGE_STARTUP || stage == PHP_INI_STAGE_SHUTDOWN || stage == PHP_INI_STAGE_ACTIVATE || stage == PHP_INI_STAGE_DEACTIVATE) { + if (PG(open_basedir_modified)) { + efree(*p); + } /* We're in a PHP_INI_SYSTEM context, no restrictions */ *p = new_value ? ZSTR_VAL(new_value) : NULL; + PG(open_basedir_modified) = false; return SUCCESS; } @@ -117,15 +121,13 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir) efree(pathbuf); /* Everything checks out, set it */ - if (*p) { - /* Unfortunately entry->modified has already been set to true so we compare entry->value - * against entry->orig_value. */ - if (entry->modified && entry->value != entry->orig_value) { - efree(*p); - } - } zend_string *tmp = smart_str_extract(&buf); - *p = estrdup(ZSTR_VAL(tmp)); + char *result = estrdup(ZSTR_VAL(tmp)); + if (PG(open_basedir_modified)) { + efree(*p); + } + *p = result; + PG(open_basedir_modified) = true; zend_string_release(tmp); return SUCCESS; diff --git a/main/php_globals.h b/main/php_globals.h index d62516f9d68..b2f2696c2db 100644 --- a/main/php_globals.h +++ b/main/php_globals.h @@ -80,6 +80,7 @@ struct _php_core_globals { char *user_dir; char *include_path; char *open_basedir; + bool open_basedir_modified; char *extension_dir; char *php_binary; char *sys_temp_dir;