Skip to content

ext/dom: Fix use-after-free in getElementById() after removing an ID attribute - #460

Closed
iliaal wants to merge 1 commit into
PHP-8.4from
fix/dom-removed-id-attr-uaf
Closed

iliaal wants to merge 1 commit into
PHP-8.4from
fix/dom-removed-id-attr-uaf

Conversation

@iliaal

@iliaal iliaal commented Nov 9, 2026

Copy link
Copy Markdown
Owner

When a detached ID attribute is freed, php_libxml_node_free_resource() freed its children before xmlFreeProp() called xmlRemoveID(). xmlRemoveID() reads the ID value from those children, so it found no entry and the document's ID table kept pointing at the freed attribute. A later getElementById() for that ID then read freed memory. Remove the ID before freeing the children, as php_libxml_node_free_list() already does.

Reproduced on PHP-8.4 under ASAN with xml:id, DTD-declared IDs and Dom\XMLDocument. removeAttribute() frees the attribute with its children intact, so it is not affected. Found while fixing the XPath callback use-after-free in the follow-up PR.

…attribute

php_libxml_node_free_resource() freed a detached ID attribute's children
before xmlFreeProp() called xmlRemoveID(), which reads the ID value from
those children, so the entry was not found and the document's ID table kept
pointing at the freed attribute. Remove the ID before freeing the children,
as php_libxml_node_free_list() already does. This also covers
removeAttribute() while a reference to the attribute node is still held.
@iliaal
iliaal force-pushed the fix/dom-removed-id-attr-uaf branch from b88d60d to 0726b06 Compare November 9, 2026 19:20
@iliaal

iliaal commented Nov 9, 2026

Copy link
Copy Markdown
Owner Author

Submitted upstream as php#25351.

@iliaal iliaal closed this Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant