Skip to content

ext/dom: php_dom_free_templated_content() use-after-free. - #23341

Open
devnexen wants to merge 2 commits into
php:PHP-8.4from
devnexen:gh23334
Open

ext/dom: php_dom_free_templated_content() use-after-free.#23341
devnexen wants to merge 2 commits into
php:PHP-8.4from
devnexen:gh23334

Conversation

@devnexen

@devnexen devnexen commented Aug 18, 2026

Copy link
Copy Markdown
Member

Fix #23334

The fragment was freed with xmlFreeNode(), assuming no userland reference into template contents could exist. getElementById() reaches those nodes, so freeing the template element left dangling proxies.

The children now go through php_libxml_node_free_list(), and the fragment is freed only when it has no proxy; otherwise its parent link is cleared so it is freed with the last reference.

Fix php#23334

The fragment was freed with xmlFreeNode(), assuming no userland
reference into template contents could exist. getElementById() reaches
those nodes, so freeing the template element left dangling proxies.

The children now go through php_libxml_node_free_list(), and the
fragment is freed only when it has no proxy; otherwise its parent link
is cleared so it is freed with the last reference.
Upward walks followed the fragment's parent link to its host, placing
template contents in the host document instead of rooting them at the
fragment. They now stop at a fragment via php_dom_parent_node(), except
the cycle check which the spec resolves against host-including ancestors.
@devnexen

Copy link
Copy Markdown
Member Author

There can be two more changes (e.g. Dom\HTMLTemplateElement) but they are more master material. @ndossche let me know if even the 2nd commit is too.

@devnexen
devnexen marked this pull request as ready for review August 18, 2026 07:37
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.

1 participant