Fix GH-12621: browscap segmentation fault when configured in the vhost

The temporary HashTable has a destructor that releases the string held
by the entry's value. However, browscap_intern_str(_ci) only incremented
the refcount for the reference created by the return value. As the
HashTable is only used during parsing, we don't need to manage the
reference count of the value anyway, so get rid of the destructor.

This is triggerable in two cases:
 - When using php_admin_value to set the ini at the activation stage
 - When running out of space for the opcache-interned strings

Closes GH-12634.
This commit is contained in:
Niels Dossche 2023-11-08 20:06:18 +01:00
parent 333cf3c111
commit 7353c7ce17
3 changed files with 52 additions and 6 deletions

2
NEWS
View File

@ -10,6 +10,8 @@ PHP NEWS
- Standard:
. Fix memory leak in syslog device handling. (danog)
. Fixed bug GH-12621 (browscap segmentation fault when configured in the
vhost). (nielsdos)
- SQLite3:
. Fixed bug GH-12633 (sqlite3_defensive.phpt fails with sqlite 3.44.0).

View File

@ -228,7 +228,7 @@ static zend_string *browscap_intern_str(
} else {
interned = zend_string_copy(str);
if (persistent) {
interned = zend_new_interned_string(str);
interned = zend_new_interned_string(interned);
}
zend_hash_add_new_ptr(&ctx->str_interned, interned, interned);
}
@ -397,10 +397,6 @@ static void php_browscap_parser_cb(zval *arg1, zval *arg2, zval *arg3, int callb
}
/* }}} */
static void str_interned_dtor(zval *zv) {
zend_string_release(Z_STR_P(zv));
}
static int browscap_read_file(char *filename, browser_data *browdata, int persistent) /* {{{ */
{
zend_file_handle fh;
@ -430,7 +426,9 @@ static int browscap_read_file(char *filename, browser_data *browdata, int persis
ctx.bdata = browdata;
ctx.current_entry = NULL;
ctx.current_section_name = NULL;
zend_hash_init(&ctx.str_interned, 8, NULL, str_interned_dtor, persistent);
/* No dtor because we don't inc the refcount for the reference stored within the hash table's entry value
* as the hash table is only temporary anyway. */
zend_hash_init(&ctx.str_interned, 8, NULL, NULL, persistent);
zend_parse_ini_file(&fh, persistent, ZEND_INI_SCANNER_RAW,
(zend_ini_parser_cb_t) php_browscap_parser_cb, &ctx);

View File

@ -0,0 +1,46 @@
--TEST--
GH-12621 (browscap segmentation fault when configured with php_admin_value)
--SKIPIF--
<?php include "skipif.inc"; ?>
--FILE--
<?php
require_once "tester.inc";
$cfg = <<<EOT
[global]
error_log = {{FILE:LOG}}
[unconfined]
listen = {{ADDR}}
pm = dynamic
pm.max_children = 5
pm.start_servers = 1
pm.min_spare_servers = 1
pm.max_spare_servers = 3
EOT;
$cfg .= 'php_admin_value[browscap] = ' . __DIR__ . '/../../../ext/standard/tests/misc/browscap.ini';
$code = <<<EOT
<?php
\$cv = get_browser("Konqueror/2.0")->browser_name_pattern;
var_dump(\$cv);
EOT;
$tester = new FPM\Tester($cfg, $code);
$tester->start();
$tester->expectLogStartNotices();
echo $tester
->request()
->getBody();
$tester->terminate();
$tester->close();
?>
--EXPECT--
string(14) "*Konqueror/2.*"
--CLEAN--
<?php
require_once "tester.inc";
FPM\Tester::clean();
?>