Skip to content

ext/libxml: reject changing the external entity loader while it is running - #23826

Open
jvoisin wants to merge 1 commit into
php:masterfrom
jvoisin:callback_xml
Open

jvoisin wants to merge 1 commit into
php:masterfrom
jvoisin:callback_xml

Conversation

@jvoisin

@jvoisin jvoisin commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

The loader callback ran with a borrowed $this (non-closures are not ref-held by zend_call_function), so re-registering the loader from within its own callback freed the executing callback and its bound object, causing a use-after-free. Guard it with an in-callback flag and throw instead, as already done in ext/curl, ext/sqlite3 and ext/xml.

@devnexen

Copy link
Copy Markdown
Member

can we have the following tests ?

--TEST--
libxml_set_external_entity_loader(): the in-callback state is reset when the callback bails out
--EXTENSIONS--
dom
--INI--
memory_limit=8M
--FILE--
<?php
register_shutdown_function(function () {
    try {
        libxml_set_external_entity_loader(null);
        echo 'loader reset', PHP_EOL;
    } catch (\Error $e) {
        echo $e::class, ': ', $e->getMessage(), PHP_EOL;
    }
});

$loader = function ($public, $system, $context) {
    echo 'in loader', PHP_EOL;
    $s = str_repeat('a', 1024 * 1024 * 512);
};

libxml_set_external_entity_loader($loader);

$xml = <<<XML
<!DOCTYPE foo PUBLIC "-//FOO/BAR" "http://example.com/foobar">
<foo>bar</foo>
XML;

$dd = new DOMDocument;
$dd->loadXML($xml);
$dd->validate();

echo 'not reached', PHP_EOL;
?>
--EXPECTF--
in loader

Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d
loader reset
--TEST--
libxml_set_external_entity_loader(): the loader may swap itself out for a nested parse
--EXTENSIONS--
dom
--FILE--
<?php
$loader = null;
$loader = function ($public, $system, $context) use (&$loader) {
    libxml_set_external_entity_loader(null);
    $inner = new DOMDocument;
    $inner->loadXML('<inner/>');
    echo $inner->documentElement->nodeName, PHP_EOL;
    libxml_set_external_entity_loader($loader);

    $f = fopen('php://temp', 'r+');
    fwrite($f, '<!ELEMENT foo (#PCDATA)>');
    rewind($f);
    return $f;
};

libxml_set_external_entity_loader($loader);

$xml = <<<XML
<!DOCTYPE foo PUBLIC "-//FOO/BAR" "http://example.com/foobar">
<foo>bar</foo>
XML;

$dd = new DOMDocument;
$dd->loadXML($xml);
var_dump($dd->validate());

libxml_set_external_entity_loader(null);
var_dump(libxml_get_external_entity_loader());

echo 'Done.', PHP_EOL;
?>
--EXPECT--
inner
bool(true)
NULL
Done.

@jvoisin

jvoisin commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Sure

Comment thread ext/libxml/libxml.c Outdated
…from its own callback

The loader callback ran with a borrowed $this (non-closures are not
ref-held by zend_call_known_fcc), so re-registering the loader from within
its own callback freed the executing callback and its bound object,
causing a use-after-free.

Hold a reference to the callback for the duration of the call instead, so
the loader may safely swap itself out (for instance to run a nested parse)
while it is running.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants