[php-src] master: Fix use-after-free when __clone() retains the stylesheet copy

Author: Ilia Alshanetsky (iliaal) Date: 2026-08-10T15:04:54-04:00 Commit: https://github.com/php/php-src/commit/11047a7a6688b2360c3c3a26370caa39862a2587 Raw diff: https://github.com/php/php-src/commit/11047a7a6688b2360c3c3a26370caa39862a2587.diff Fix use-after-free when __clone() retains the stylesheet 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. Closes GH-23199 Changed paths: A ext/xsl/tests/importStylesheet_clone_retained_document.phpt A ext/xsl/tests/importStylesheet_clone_retained_node.phpt M NEWS M ext/xsl/xsltprocessor.c Diff: diff --git a/NEWS b/NEWS index d19e6b8ae2af..6447fa7bc881 100644 --- a/NEWS +++ b/NEWS @@ -87,6 +87,10 @@ PHP NEWS . Fixed out-of-bounds write when shm_attach() opens an existing segment with a size larger than the segment actually is. (David Carlier) +- XSL: + . Fixed use-after-free when a DOMDocument subclass __clone() retains the + stylesheet copy made by XSLTProcessor::importStylesheet(). (iliaal) + - Zip: . Fixed ZipArchive::addGlob() and ZipArchive::addPattern() ignoring their default options when no options array is given. (David Carlier) diff --git a/ext/xsl/tests/importStylesheet_clone_retained_document.phpt b/ext/xsl/tests/importStylesheet_clone_retained_document.phpt new file mode 100644 index 000000000000..481925677d4e --- /dev/null +++ b/ext/xsl/tests/importStylesheet_clone_retained_document.phpt @@ -0,0 +1,47 @@ +--TEST-- +XSLTProcessor::importStylesheet() rejects a stylesheet whose __clone() retains the cloned document +--EXTENSIONS-- +dom +xsl +--FILE-- +<?php +const STYLESHEET = <<<XML +<?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> +XML; + +class Harmless extends DOMDocument { + public function __clone(): void { + } +} + +class RetainsDocument extends DOMDocument { + public function __clone(): void { + $GLOBALS['stash'] = $this; + } +} + +$doc = new Harmless; +$doc->loadXML(STYLESHEET); +$proc = new XSLTProcessor(); +var_dump($proc->importStylesheet($doc)); +unset($proc, $doc); + +$doc = new RetainsDocument; +$doc->loadXML(STYLESHEET); +$proc = new XSLTProcessor(); +try { + var_dump($proc->importStylesheet($doc)); +} catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} +$kept = $GLOBALS['stash']; +unset($GLOBALS['stash'], $proc, $doc); +echo get_class($kept), " is still usable: ", $kept->documentElement->nodeName, PHP_EOL; +?> +--EXPECT-- +bool(true) +ValueError: XSLTProcessor::importStylesheet(): Argument #1 ($stylesheet) must not have its clone retained by __clone() +RetainsDocument is still usable: xsl:stylesheet diff --git a/ext/xsl/tests/importStylesheet_clone_retained_node.phpt b/ext/xsl/tests/importStylesheet_clone_retained_node.phpt new file mode 100644 index 000000000000..72c47b73b002 --- /dev/null +++ b/ext/xsl/tests/importStylesheet_clone_retained_node.phpt @@ -0,0 +1,34 @@ +--TEST-- +XSLTProcessor::importStylesheet() rejects a stylesheet whose __clone() retains a node of the cloned document +--EXTENSIONS-- +dom +xsl +--FILE-- +<?php +class RetainsElement extends DOMDocument { + public function __clone(): void { + $GLOBALS['stash'] = $this->documentElement; + } +} + +$doc = new RetainsElement; +$doc->loadXML(<<<XML +<?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> +XML); + +$proc = new XSLTProcessor(); +try { + var_dump($proc->importStylesheet($doc)); +} catch (Error $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} +$kept = $GLOBALS['stash']; +unset($GLOBALS['stash'], $proc, $doc); +echo get_class($kept), " is still usable: ", $kept->nodeName, PHP_EOL; +?> +--EXPECT-- +ValueError: XSLTProcessor::importStylesheet(): Argument #1 ($stylesheet) must not have its clone retained by __clone() +DOMElement is still usable: xsl:stylesheet diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index 71971332a251..cf5a941d95ca 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -227,6 +227,12 @@ PHP_METHOD(XSLTProcessor, importStylesheet) php_libxml_node_object *clone_lxml_obj = Z_LIBXML_NODE_P(&clone_zv); + if (GC_REFCOUNT(clone) > 1 || clone_lxml_obj->document->refcount > 1) { + OBJ_RELEASE(clone); + zend_argument_value_error(1, "must not have its clone retained by __clone()"); + RETURN_THROWS(); + } + PHP_LIBXML_SANITIZE_GLOBALS(parse); ZEND_DIAGNOSTIC_IGNORED_START("-Wdeprecated-declarations") xmlSubstituteEntitiesDefault(1);