Skip to content

Commit 127ad70

Browse files
committed
Fix open_basedir leak
Fixes oss-fuzz #60741 Closes GH-11780
1 parent 61d16a6 commit 127ad70

File tree

5 files changed

+23
-26
lines changed

5 files changed

+23
-26
lines changed

NEWS

+3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.3.0beta2
44

5+
- Core:
6+
. Fixed oss-fuzz #60741 (Leak in open_basedir). (ilutov)
7+
58
- FFI:
69
. Fix leaking definitions when using FFI::cdef()->new(...). (ilutov)
710

Zend/tests/oss_fuzz_60741.phpt

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
--TEST--
2+
oss-fuzz #60741: Leak in open_basedir
3+
--INI--
4+
open_basedir="{TMP}"
5+
--FILE--
6+
<?php
7+
ini_set('open_basedir', ini_get('open_basedir'));
8+
?>
9+
--EXPECT--

Zend/zend.c

-18
Original file line numberDiff line numberDiff line change
@@ -1271,29 +1271,11 @@ void zend_call_destructors(void) /* {{{ */
12711271
}
12721272
/* }}} */
12731273

1274-
static void zend_release_open_basedir(void)
1275-
{
1276-
/* Release custom open_basedir config, this needs to happen before ini shutdown */
1277-
if (PG(open_basedir)) {
1278-
zend_ini_entry *ini_entry = zend_hash_str_find_ptr(EG(ini_directives), "open_basedir", strlen("open_basedir"));
1279-
/* ini_entry->modified is unreliable, it might also be set when on_update has failed. */
1280-
if (ini_entry
1281-
&& ini_entry->modified
1282-
&& ini_entry->value != ini_entry->orig_value) {
1283-
efree(PG(open_basedir));
1284-
PG(open_basedir) = NULL;
1285-
}
1286-
}
1287-
}
1288-
12891274
ZEND_API void zend_deactivate(void) /* {{{ */
12901275
{
12911276
/* we're no longer executing anything */
12921277
EG(current_execute_data) = NULL;
12931278

1294-
/* Needs to run before zend_ini_deactivate(). */
1295-
zend_release_open_basedir();
1296-
12971279
zend_try {
12981280
shutdown_scanner();
12991281
} zend_end_try();

main/fopen_wrappers.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,12 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
7777
char *pathbuf, *ptr, *end;
7878

7979
if (stage == PHP_INI_STAGE_STARTUP || stage == PHP_INI_STAGE_SHUTDOWN || stage == PHP_INI_STAGE_ACTIVATE || stage == PHP_INI_STAGE_DEACTIVATE) {
80+
if (PG(open_basedir_modified)) {
81+
efree(*p);
82+
}
8083
/* We're in a PHP_INI_SYSTEM context, no restrictions */
8184
*p = new_value ? ZSTR_VAL(new_value) : NULL;
85+
PG(open_basedir_modified) = false;
8286
return SUCCESS;
8387
}
8488

@@ -117,15 +121,13 @@ PHPAPI ZEND_INI_MH(OnUpdateBaseDir)
117121
efree(pathbuf);
118122

119123
/* Everything checks out, set it */
120-
if (*p) {
121-
/* Unfortunately entry->modified has already been set to true so we compare entry->value
122-
* against entry->orig_value. */
123-
if (entry->modified && entry->value != entry->orig_value) {
124-
efree(*p);
125-
}
126-
}
127124
zend_string *tmp = smart_str_extract(&buf);
128-
*p = estrdup(ZSTR_VAL(tmp));
125+
char *result = estrdup(ZSTR_VAL(tmp));
126+
if (PG(open_basedir_modified)) {
127+
efree(*p);
128+
}
129+
*p = result;
130+
PG(open_basedir_modified) = true;
129131
zend_string_release(tmp);
130132

131133
return SUCCESS;

main/php_globals.h

+1
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ struct _php_core_globals {
8080
char *user_dir;
8181
char *include_path;
8282
char *open_basedir;
83+
bool open_basedir_modified;
8384
char *extension_dir;
8485
char *php_binary;
8586
char *sys_temp_dir;

0 commit comments

Comments
 (0)