Skip to content

Commit e173e6d

Browse files
committed
Fix GH-24763: Handle libxml entity loader destruction reentrancy
1 parent c5003a7 commit e173e6d

3 files changed

Lines changed: 84 additions & 5 deletions

File tree

‎ext/libxml/libxml.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,7 +1041,10 @@ static PHP_RINIT_FUNCTION(libxml)
10411041
static PHP_RSHUTDOWN_FUNCTION(libxml)
10421042
{
10431043
if (ZEND_FCC_INITIALIZED(LIBXML(entity_loader_callback))) {
1044-
zend_fcc_dtor(&LIBXML(entity_loader_callback));
1044+
/* Detach before destruction, as destroying the callback may bail out. */
1045+
zend_fcall_info_cache old_fcc = LIBXML(entity_loader_callback);
1046+
LIBXML(entity_loader_callback) = empty_fcall_info_cache;
1047+
zend_fcc_dtor(&old_fcc);
10451048
}
10461049

10471050
return SUCCESS;
@@ -1261,13 +1264,18 @@ PHP_FUNCTION(libxml_set_external_entity_loader)
12611264
Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc)
12621265
ZEND_PARSE_PARAMETERS_END();
12631266

1264-
/* Unset old callback if it's defined */
1265-
if (ZEND_FCC_INITIALIZED(LIBXML(entity_loader_callback))) {
1266-
zend_fcc_dtor(&LIBXML(entity_loader_callback));
1267-
}
1267+
/* Detach the old callback before destruction, which may re-enter this function. */
1268+
zend_fcall_info_cache old_fcc = LIBXML(entity_loader_callback);
1269+
LIBXML(entity_loader_callback) = empty_fcall_info_cache;
1270+
12681271
if (ZEND_FCI_INITIALIZED(fci)) { /* argument not null */
12691272
zend_fcc_dup(&LIBXML(entity_loader_callback), &fcc);
12701273
}
1274+
1275+
/* Destroy only after installing the replacement so re-entrant changes are preserved. */
1276+
if (ZEND_FCC_INITIALIZED(old_fcc)) {
1277+
zend_fcc_dtor(&old_fcc);
1278+
}
12711279
RETURN_TRUE;
12721280
}
12731281
/* }}} */
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
GH-24763: libxml external entity loader is detached before callback destruction bailout
3+
--EXTENSIONS--
4+
libxml
5+
zend_test
6+
--FILE--
7+
<?php
8+
9+
class Bailout
10+
{
11+
public function __destruct()
12+
{
13+
zend_trigger_bailout();
14+
}
15+
}
16+
17+
$bailout = new Bailout();
18+
19+
$loader = function () use ($bailout) {
20+
};
21+
22+
unset($bailout);
23+
24+
register_shutdown_function(function () {
25+
var_dump(libxml_get_external_entity_loader());
26+
});
27+
28+
libxml_set_external_entity_loader($loader);
29+
unset($loader);
30+
31+
libxml_set_external_entity_loader(null);
32+
33+
echo "UNREACHABLE\n";
34+
35+
?>
36+
--EXPECTF--
37+
Fatal error: Bailout in %s on line %d
38+
NULL
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
libxml external entity loader replacement is safe against destructor reentrancy
3+
--EXTENSIONS--
4+
libxml
5+
--FILE--
6+
<?php
7+
8+
class D
9+
{
10+
public function __destruct()
11+
{
12+
libxml_set_external_entity_loader(fn() => null);
13+
}
14+
}
15+
16+
$d = new D();
17+
18+
$oldLoader = function () use ($d) {
19+
};
20+
21+
unset($d);
22+
23+
libxml_set_external_entity_loader($oldLoader);
24+
unset($oldLoader);
25+
26+
libxml_set_external_entity_loader(fn() => 1);
27+
28+
$loader = libxml_get_external_entity_loader();
29+
var_dump($loader());
30+
31+
?>
32+
--EXPECT--
33+
NULL

0 commit comments

Comments
 (0)