Skip to content

Cherry-pick(s) 2e63eaf64497. rdar://191051699 - #78737

Closed
webkit-opensource-integration wants to merge 1 commit into
mainfrom
integration/conflict/2e63eaf64497_2e63eaf64497_main
Closed

webkit-opensource-integration wants to merge 1 commit into
mainfrom
integration/conflict/2e63eaf64497_2e63eaf64497_main

Conversation

@webkit-opensource-integration

@webkit-opensource-integration webkit-opensource-integration commented Oct 29, 2026 •

Copy link
Copy Markdown

Integration has hit a merge conflict while cherry-picking rdar://191051699 to main. [2e63eaf] caused a merge conflict.

Use-after-free in reused inner/outerHTML DocumentFragment
https://bugs.webkit.org/show_bug.cgi?id=322242
rdar://183463022

Reviewed by Wenson Hsieh.

The setters of innerHTML and outerHTML re-uses the same document fragment for parsing purposes
as a performance optimization. There is a further performance optimization in
ContainerNode::removeAllChildrenWithScriptAssertion to avoid calling Node::removingSteps on the
freshly parsed DOM tree under the document fragment for innerHTML and outerHTML. This leaves
Node::m_shadowIncludingRoot to remain pointing to the document fragment even after they had
logically been removed from the fragment.

This is ordinarily fine because the newly parsed DOM tree gets inserted into the context object
of innerHTML and outerHTML right away so insertionSteps will update Node::m_shadowIncludingRoot.

However, when a XML parser encounters a parsing error, it pauses the process midway and exit
early before Element::setOuterHTML had a chance to insert the new DOM tree to the context object.
If XML parser had constructed any custom elements by that point, those constructors will still
get called while the elements are a part of the reused document fragment, exposing it to scripts.

When script then removes the node from the document fragment, the performance optimization to
avoid calling Node::removingSteps in ContainerNode::removeAllChildrenWithScriptAssertion kicks in
and leaves DOM tree in an inconsistent state with a stale pointer in Node::m_shadowIncludingRoot.

Fix the bug by disabling the document fragment reuse when parsing XML. XML parser isn't optimized
for performance anyway.

Test: fast/dom/xml-parser-error-during-inner-outer-html-crash.html

* LayoutTests/fast/dom/xml-parser-error-during-inner-outer-html-crash-expected.txt: Added.
* LayoutTests/fast/dom/xml-parser-error-during-inner-outer-html-crash.html: Added.
* Source/WebCore/editing/markup.cpp:
(WebCore::createFragmentForMarkup):

Originally-landed-as: 316606.427@safari-7625-branch (2e63eaf64497). rdar://191051699


ℹ️ Info

RC_REMOVE_SEC_ACCEL_* guards were removed with unifdef before cherry-picking, keeping only the code built when the macro is not defined.

6b1ac92

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe ❌ 🛠 win
❌ 🧪 bindings ❌ 🛠 ios-sim ❌ 🛠 mac-cmake-debug ❌ 🧪 wpe-wk2 ❌ 🧪 win-tests
❌ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ❌ 🧪 api-mac-cmake-debug ❌ 🧪 jsc-wpe
❌ 🧪 jsc-x86-64 ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🛠 gtk3-gcc
❌ 🛠 ios-safer-cpp ❌ 🧪 mac-cmake-debug-wk2 ❌ 🛠 gtk
❌ 🛠 vision ❌ 🧪 mac-wk2-stress ❌ 🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-intel-wk2 ❌ 🧪 api-gtk
❌ 🧪 vision-wk2 ❌ 🛠 mac-safer-cpp ❌ 🛠 playstation
❌ 🛠 tv ❌ 🧪 mac-site-isolation
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=322242
rdar://183463022

Reviewed by Wenson Hsieh.

The setters of innerHTML and outerHTML re-uses the same document fragment for parsing purposes
as a performance optimization. There is a further performance optimization in
ContainerNode::removeAllChildrenWithScriptAssertion to avoid calling Node::removingSteps on the
freshly parsed DOM tree under the document fragment for innerHTML and outerHTML. This leaves
Node::m_shadowIncludingRoot to remain pointing to the document fragment even after they had
logically been removed from the fragment.

This is ordinarily fine because the newly parsed DOM tree gets inserted into the context object
of innerHTML and outerHTML right away so insertionSteps will update Node::m_shadowIncludingRoot.

However, when a XML parser encounters a parsing error, it pauses the process midway and exit
early before Element::setOuterHTML had a chance to insert the new DOM tree to the context object.
If XML parser had constructed any custom elements by that point, those constructors will still
get called while the elements are a part of the reused document fragment, exposing it to scripts.

When script then removes the node from the document fragment, the performance optimization to
avoid calling Node::removingSteps in ContainerNode::removeAllChildrenWithScriptAssertion kicks in
and leaves DOM tree in an inconsistent state with a stale pointer in Node::m_shadowIncludingRoot.

Fix the bug by disabling the document fragment reuse when parsing XML. XML parser isn't optimized
for performance anyway.

Test: fast/dom/xml-parser-error-during-inner-outer-html-crash.html

* LayoutTests/fast/dom/xml-parser-error-during-inner-outer-html-crash-expected.txt: Added.
* LayoutTests/fast/dom/xml-parser-error-during-inner-outer-html-crash.html: Added.
* Source/WebCore/editing/markup.cpp:
(WebCore::createFragmentForMarkup):

Originally-landed-as: 316606.427@safari-7625-branch (2e63eaf). rdar://191051699
@webkit-opensource-integration

Copy link
Copy Markdown
Author

Integration performed the following cherry-picks: 2e63eaf for rdar://191051699. 2e63eaf caused a merge conflict.

@webkit-opensource-integration

Copy link
Copy Markdown
Author

At the time this pull request was created the following files were in conflict and need to be resolved:

  • Source/WebCore/editing/markup.cpp — modified on both the target branch and the incoming commit (content conflict)

@rniwa

rniwa commented Oct 29, 2026

Copy link
Copy Markdown
Member

We should instead merge rdar://191058513.

@rniwa rniwa closed this Oct 29, 2026
@rniwa
rniwa deleted the integration/conflict/2e63eaf64497_2e63eaf64497_main branch October 29, 2026 18:03
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.

3 participants