Skip to content

Commit e1c6a7c

Browse files
nielsdosramsey
authored andcommitted
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.
1 parent 6641cd1 commit e1c6a7c

File tree

3 files changed

+52
-6
lines changed

3 files changed

+52
-6
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ PHP NEWS
1010

1111
- Standard:
1212
. Fix memory leak in syslog device handling. (danog)
13+
. Fixed bug GH-12621 (browscap segmentation fault when configured in the
14+
vhost). (nielsdos)
1315

1416
- SQLite3:
1517
. Fixed bug GH-12633 (sqlite3_defensive.phpt fails with sqlite 3.44.0).

ext/standard/browscap.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static zend_string *browscap_intern_str(
228228
} else {
229229
interned = zend_string_copy(str);
230230
if (persistent) {
231-
interned = zend_new_interned_string(str);
231+
interned = zend_new_interned_string(interned);
232232
}
233233
zend_hash_add_new_ptr(&ctx->str_interned, interned, interned);
234234
}
@@ -397,10 +397,6 @@ static void php_browscap_parser_cb(zval *arg1, zval *arg2, zval *arg3, int callb
397397
}
398398
/* }}} */
399399

400-
static void str_interned_dtor(zval *zv) {
401-
zend_string_release(Z_STR_P(zv));
402-
}
403-
404400
static int browscap_read_file(char *filename, browser_data *browdata, int persistent) /* {{{ */
405401
{
406402
zend_file_handle fh;
@@ -430,7 +426,9 @@ static int browscap_read_file(char *filename, browser_data *browdata, int persis
430426
ctx.bdata = browdata;
431427
ctx.current_entry = NULL;
432428
ctx.current_section_name = NULL;
433-
zend_hash_init(&ctx.str_interned, 8, NULL, str_interned_dtor, persistent);
429+
/* No dtor because we don't inc the refcount for the reference stored within the hash table's entry value
430+
* as the hash table is only temporary anyway. */
431+
zend_hash_init(&ctx.str_interned, 8, NULL, NULL, persistent);
434432

435433
zend_parse_ini_file(&fh, persistent, ZEND_INI_SCANNER_RAW,
436434
(zend_ini_parser_cb_t) php_browscap_parser_cb, &ctx);

sapi/fpm/tests/gh12621.phpt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
GH-12621 (browscap segmentation fault when configured with php_admin_value)
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
[unconfined]
14+
listen = {{ADDR}}
15+
pm = dynamic
16+
pm.max_children = 5
17+
pm.start_servers = 1
18+
pm.min_spare_servers = 1
19+
pm.max_spare_servers = 3
20+
21+
EOT;
22+
$cfg .= 'php_admin_value[browscap] = ' . __DIR__ . '/../../../ext/standard/tests/misc/browscap.ini';
23+
24+
$code = <<<EOT
25+
<?php
26+
\$cv = get_browser("Konqueror/2.0")->browser_name_pattern;
27+
var_dump(\$cv);
28+
EOT;
29+
30+
$tester = new FPM\Tester($cfg, $code);
31+
$tester->start();
32+
$tester->expectLogStartNotices();
33+
echo $tester
34+
->request()
35+
->getBody();
36+
$tester->terminate();
37+
$tester->close();
38+
39+
?>
40+
--EXPECT--
41+
string(14) "*Konqueror/2.*"
42+
--CLEAN--
43+
<?php
44+
require_once "tester.inc";
45+
FPM\Tester::clean();
46+
?>

0 commit comments

Comments
 (0)