Skip to content

Fix use-after-free when __clone() retains the stylesheet copy - #23199

Open
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/xsl-importstylesheet-clone-escape
Open

Fix use-after-free when __clone() retains the stylesheet copy#23199
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/xsl-importstylesheet-clone-escape

Conversation

@iliaal

@iliaal iliaal commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

XSLTProcessor::importStylesheet() clones the stylesheet document through the object's clone handler and hands the copy to libxslt, which frees it with the stylesheet. A DOMDocument subclass __clone() runs while that copy is fully wired up and can stash either the copy itself or a node proxy into it, leaving a dangling pointer once the processor is destroyed. The clone must now be exclusively owned before libxslt takes it.

class D extends DOMDocument {
    public function __clone(): void { $GLOBALS['stash'] = $this; }
}
$d = new D;
$d->loadXML('<?xml version="1.0"?><xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform"><xsl:template match="/"><out/></xsl:template></xsl:stylesheet>');
$p = new XSLTProcessor;
$p->importStylesheet($d);
$esc = $GLOBALS['stash'];
unset($GLOBALS['stash'], $p, $d);

That aborts with a double free on 8.4, 8.5 and master. Stashing $this->documentElement instead gives a silent read of freed memory in dom_objects_free_storage. 8.3 is unaffected because it copies at the libxml level and never builds a PHP object for the copy.

importStylesheet() clones the stylesheet document and hands the copy to
libxslt, which owns it and frees it together with the stylesheet. The
clone goes through zend_objects_clone_members(), so a DOMDocument
subclass __clone() can retain the copy, or a node proxy into it, and
dereference freed memory once the processor is destroyed. Require the
clone to be exclusively owned before libxslt takes it.
Comment thread ext/xsl/xsltprocessor.c

if (GC_REFCOUNT(clone) > 1 || clone_lxml_obj->document->refcount > 1) {
OBJ_RELEASE(clone);
zend_throw_error(NULL, "XSLTProcessor::importStylesheet(): Argument #1 ($stylesheet) must not have its clone retained by __clone()");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after giving some thoughts, zend_argument_value_error is more appropriate here.

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