CVE-2024-44192
Overview
Background
- Ruby / RubyBlock rendering
- CSS ruby lays out annotation text alongside base text; WebKit uses a RubyBlock renderer that wraps ruby children in an anonymous Ruby-display box.
- Render-tree continuation
- The mechanism WebKit uses when an inline box contains a block-level box: the inline is split and linked via continuations, inserting extra anonymous wrapper renderers into the tree.
- Anonymous renderer
- A renderer with no corresponding DOM element that the engine synthesizes to satisfy box-model invariants (e.g. the anonymous ruby wrapper).
- firstChildSlow()
- A RenderObject accessor that returns the first child, used here to descend the first-child chain layer by layer while searching for the anonymous ruby box.
- ASSERT vs release behavior
- ASSERT/ASSERT_NOT_REACHED abort in debug builds but compile to nothing in release, so a violated ASSERT becomes silent wrong behavior (here, a bad downcast) in shipping builds.
Root Cause Analysis
The bug is in RenderTreeBuilder::Ruby::findOrCreateParentForStyleBasedRubyChild in Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp, which, when appending a child under a RubyBlock parent, tries to locate the existing anonymous ruby box that should wrap ruby children. The pre-fix code did ‘if (parent.style().display() == DisplayType::RubyBlock && parent.firstChild())’ and then ASSERTed that ‘parent.firstChild()->style().display() == DisplayType::Ruby’ before returning parent.firstChild() downcast to RenderElement. The violated invariant is that the RubyBlock’s immediate first child is always the anonymous Ruby-display box. That assumption is false in the presence of render-tree continuations: when inline content (the ruby base) contains a block-level box (the <div> inside the <span> inside <rb> in the test), the inline is split and continuations are inserted, so the RubyBlock’s first child may be some other anonymous wrapper rather than the expected Ruby box. In a release build the ASSERT is compiled out, so the code would unconditionally downcast and return a first child that is not the anonymous ruby box, wiring subsequent children into the wrong renderer and corrupting the render tree (leading to the crash the test guards against); in a debug build the ASSERT fires.
The fix replaces the single-firstChild check with a descent loop: for (CheckedPtr first = parent.firstChild(); first; first = first->firstChildSlow()) it walks down the first-child chain, bailing with ASSERT_NOT_REACHED if it ever encounters a non-anonymous box, and returns the first box whose display is Ruby. This restores the invariant by actually searching for the anonymous ruby box through the continuation-introduced wrapper layers instead of assuming it is the immediate first child, and the accompanying FIXME explicitly notes ‘It should be the immediate child but continuations can break this assumption.’ The regression test (<ruby style=‘position: absolute’> with an <rb><span>…<div>forced</div>…</span></rb>) reproduces exactly the continuation-splitting scenario and passes if it does not crash.
Attack Path
- Build a ruby subtree that forces continuations Author HTML where a ruby base contains inline content wrapping a block-level element (e.g. <ruby><rb><span>text <div>forced</div> break</span></rb><rt>…</rt></ruby>), so the inline is split and continuations are inserted.
- Trigger RubyBlock render-tree construction Use styling such as position: absolute on the ruby to force the style-based ruby render path so findOrCreateParentForStyleBasedRubyChild runs against a RubyBlock parent.
- Reach the false first-child assumption During child insertion the code assumes parent.firstChild() is the anonymous Ruby box, but continuations have made the first child a different anonymous wrapper.
- Bad downcast / misparented child In release builds (ASSERT compiled out) the code downcasts and returns the wrong renderer as the ruby parent, corrupting the render tree.
- Provoke the crash Subsequent layout/render-tree operations on the malformed tree dereference or manipulate an unexpected node type, causing an unexpected process crash.
Impact Assessment
Files Changed
Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cppLayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txtLayoutTests/fast/ruby/ruby-block-continuation-crash.html
Audit Directions
- Other firstChild() assumptions in ruby buildingScan RenderTreeBuilderRuby.cpp and related ruby code for remaining uses of ‘firstChild()’ / ‘downcast<RenderElement>’ that assume a specific child type without a loop or isAnonymous()/display() check; verify each tolerates continuation-inserted wrappers.
- ASSERT-guarded downcasts across RenderTreeBuilderGrep the rendering/updating RenderTreeBuilder*.cpp files for the pattern of an ASSERT on style().display() or node type immediately followed by a downcast<> and return, which is the same ‘assumption compiled out in release’ shape fixed here.
- Continuation-breaking-invariants patternSearch WebCore rendering for places that assume an immediate parent/child relationship (firstChild/parent) for anonymous wrappers; grep for ‘continuation’, ‘isAnonymous’, ‘firstChildSlow’ and look for spots that inspect only the immediate child where block-in-inline splitting could insert extra layers.
Patch
diff --git a/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp b/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp
index 9259b2330dd4..fde1f46e67d7 100644
--- a/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp
+++ b/Source/WebCore/rendering/updating/RenderTreeBuilderRuby.cpp
@@ -65,10 +65,17 @@ RenderElement& RenderTreeBuilder::Ruby::findOrCreateParentForStyleBasedRubyChild
if (!child.isRenderText() && child.style().display() == DisplayType::Ruby && parent.style().display() == DisplayType::RubyBlock)
return parent;
- if (parent.style().display() == DisplayType::RubyBlock && parent.firstChild()) {
+ if (parent.style().display() == DisplayType::RubyBlock) {
// See if we have an anonymous ruby box already.
- ASSERT(parent.firstChild()->style().display() == DisplayType::Ruby);
- return downcast<RenderElement>(*parent.firstChild());
+ // FIXME: It should be the immediate child but continuations can break this assumption.
+ for (CheckedPtr first = parent.firstChild(); first; first = first->firstChildSlow()) {
+ if (!first->isAnonymous()) {
+ ASSERT_NOT_REACHED();
+ break;
+ }
+ if (first->style().display() == DisplayType::Ruby)
+ return downcast<RenderElement>(*first);
+ }
}
if (parent.style().display() != DisplayType::Ruby) {
diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt b/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt
new file mode 100644
index 000000000000..f85a15505104
--- /dev/null
+++ b/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt
@@ -0,0 +1,3 @@
+base with
+forced
+line break annotation This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash.html b/LayoutTests/fast/ruby/ruby-block-continuation-crash.html
new file mode 100644
index 000000000000..3f762d4236ea
--- /dev/null
+++ b/LayoutTests/fast/ruby/ruby-block-continuation-crash.html
@@ -0,0 +1,9 @@
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+<ruby style="position: absolute">
+ <rb><span>base with <div>forced</div> line break</span></rb>
+ <rt>annotation</rt>
+</ruby>
+This test passes if it doesn't crash.
Regression Test / PoC
diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt b/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt new file mode 100644 index 000000000000..f85a15505104 --- /dev/null +++ b/LayoutTests/fast/ruby/ruby-block-continuation-crash-expected.txt @@ -0,0 +1,3 @@ +base with +forced +line break annotation This test passes if it doesn't crash. diff --git a/LayoutTests/fast/ruby/ruby-block-continuation-crash.html b/LayoutTests/fast/ruby/ruby-block-continuation-crash.html new file mode 100644 index 000000000000..3f762d4236ea --- /dev/null +++ b/LayoutTests/fast/ruby/ruby-block-continuation-crash.html @@ -0,0 +1,9 @@ +<script> +if (window.testRunner) + testRunner.dumpAsText(); +</script> +<ruby style="position: absolute"> + <rb><span>base with <div>forced</div> line break</span></rb> + <rt>annotation</rt> +</ruby> +This test passes if it doesn't crash.