Skip to content

Commit 0af3f49

Browse files
committed
Fix #79922: Crash after multiple calls to xml_parser_free()
We must not call `zend_list_delete()` in resource closer functions exposed to userland, because decreasing the refcount there leads to use-after-free scenarios. In this case, commit 4a42fbb worked for typical use-cases where `xml_parser_free()` has been called exactly once for the resource, because there is an internal zval (`->index`) referencing the same resource which already increased the refcount by one. However, when `xml_parser_free()` is called multiple times on the same XML parser resource, the resource would be freed prematurely. Instead we forcefully close the resource in `xml_parser_free()`. We also could decrease the refcount of the resource there, but that would require to call `xml_parser_free()` which is somewhat uncommon, and would be particularly bad wrt. PHP 8 where that function is a NOP, and as such doesn't have to be called. So we do no longer increase the refcount of the resource when copying it to the internal zval, and let the usualy refcounting semantics take care of the resource destruction. [1] <http://git.php.net/?p=php-src.git;a=commit;h=4a42fbbbc73aad7427aef5c89974d1833636e082>
1 parent 5be6702 commit 0af3f49

File tree

3 files changed

+22
-2
lines changed

3 files changed

+22
-2
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ PHP NEWS
2727
. Fixed bug #79930 (array_merge_recursive() crashes when called with array
2828
with single reference). (Nikita)
2929

30+
- XML:
31+
. Fixed bug #79922 (Crash after multiple calls to xml_parser_free()). (cmb)
32+
3033
06 Aug 2020, PHP 7.3.21
3134

3235
- Apache:

ext/xml/tests/bug79922.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Bug #79922 (Crash after multiple calls to xml_parser_free())
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('xml')) die('skip xml extension not available');
6+
?>
7+
--FILE--
8+
<?php
9+
$c=xml_parser_create_ns();
10+
$a=xml_parser_free($c);
11+
$a=xml_parser_free($c);
12+
$c=0;
13+
var_dump($a);
14+
?>
15+
--EXPECTF--
16+
Warning: xml_parser_free(): supplied resource is not a valid XML Parser resource in %s on line %d
17+
bool(false)

ext/xml/xml.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,7 @@ static void php_xml_parser_create_impl(INTERNAL_FUNCTION_PARAMETERS, int ns_supp
11321132
XML_SetUserData(parser->parser, parser);
11331133

11341134
RETVAL_RES(zend_register_resource(parser, le_xml_parser));
1135-
ZVAL_COPY(&parser->index, return_value);
1135+
ZVAL_COPY_VALUE(&parser->index, return_value);
11361136
}
11371137
/* }}} */
11381138

@@ -1559,7 +1559,7 @@ PHP_FUNCTION(xml_parser_free)
15591559
RETURN_FALSE;
15601560
}
15611561

1562-
if (zend_list_delete(Z_RES(parser->index)) == FAILURE) {
1562+
if (zend_list_close(Z_RES(parser->index)) == FAILURE) {
15631563
RETURN_FALSE;
15641564
}
15651565

0 commit comments

Comments
 (0)