Bug 246771

Summary: null_ptr deref in BoxTree::layoutBoxForRenderer
Product: WebKit Reporter: Abigail F <abigail_fox>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: abigail_fox, beidson, bfulgham, cgarcia, csaavedra, darin, fred.wang, gpoo, koivisto, mmaxfield, msaboff, rbuis, simon.fraser, webkit-bug-importer, wenson_hsieh, zalan
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
repro_1017.html
none
repro_1017b.html
none
Patch
none
[fast-cq]Patch none

Description Abigail F 2022-10-19 14:30:51 PDT
Created attachment 463099 [details]
repro_1017.html

Summary:

this crashes:

<style>
  html, body {
    overflow: hidden;
  }
  ::-webkit-resizer {
    float: right;
  }
</style>
<script>
  internals.settings.setCoreMathMLEnabled(true);
</script>
<span></span>


Steps To Reproduce:

Does not reproduce in DumpRenderTree.

DYLD_FRAMEWORK_PATH=<WEBKIT_PATH>/WebKitBuild/Release <WEBKIT_PATH>/WebKitBuild/Release/WebKitTestRunner --no-enable-all-experimental-features repro_1017.html

there's something strange with this test case because for me it does not crash without the --no-enable-all-experimental-features flag

Results:

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000070
Exception Codes:       0x0000000000000001, 0x0000000000000070

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   WebCore       0x10e9fefbc WTF::OptionSet<WebCore::Layout::Box::BaseTypeFlag>::isEmpty() const + 0 (OptionSet.h:165) [inlined]
1   WebCore       0x10e9fefbc WTF::OptionSet<WebCore::Layout::Box::BaseTypeFlag>::operator bool() + 0 (OptionSet.h:170) [inlined]
2   WebCore       0x10e9fefbc WTF::OptionSet<WebCore::Layout::Box::BaseTypeFlag>::containsAny(WTF::OptionSet<WebCore::Layout::Box::BaseTypeFlag>) const + 0 (OptionSet.h:179) [inlined]
3   WebCore       0x10e9fefbc WTF::OptionSet<WebCore::Layout::Box::BaseTypeFlag>::contains(WebCore::Layout::Box::BaseTypeFlag) const + 0 (OptionSet.h:174) [inlined]
4   WebCore       0x10e9fefbc WebCore::Layout::Box::isElementBox() const + 0 (LayoutBox.h:156) [inlined]
5   WebCore       0x10e9fefbc WTF::TypeCastTraits<WebCore::Layout::ElementBox const, WebCore::Layout::Box const, false>::isType(WebCore::Layout::Box const&) + 0 (LayoutElementBox.h:111) [inlined]
6   WebCore       0x10e9fefbc WTF::TypeCastTraits<WebCore::Layout::ElementBox const, WebCore::Layout::Box const, false>::isOfType(WebCore::Layout::Box const&) + 0 (LayoutElementBox.h:111) [inlined]
7   WebCore       0x10e9fefbc bool WTF::is<WebCore::Layout::ElementBox, WebCore::Layout::Box>(WebCore::Layout::Box&) + 0 (TypeCasts.h:58) [inlined]
8   WebCore       0x10e9fefbc std::__1::conditional<std::is_const_v<WebCore::Layout::Box>, std::__1::add_const<WebCore::Layout::ElementBox>::type, std::__1::remove_const<WebCore::Layout::ElementBox>::type>::type& WTF::downcast<WebCore::Layout::ElementBox, WebCore::Layout::Box>(WebCore::Layout::Box&) + 0 (TypeCasts.h:79) [inlined]
9   WebCore       0x10e9fefbc WebCore::LayoutIntegration::BoxTree::layoutBoxForRenderer(WebCore::RenderElement const&) + 16 (LayoutIntegrationBoxTree.cpp:290)
10  WebCore       0x10ea17218 WebCore::LayoutIntegration::LineLayout::updateStyle(WebCore::RenderBoxModelObject const&, WebCore::RenderStyle const&) + 68 (LayoutIntegrationLineLayout.cpp:364)
11  WebCore       0x10eeb2c50 WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 1400 (RenderBox.cpp:441)
12  WebCore       0x10eeb21a0 WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 36 (RenderBlock.cpp:459)
13  WebCore       0x10efc2280 WebCore::RenderScrollbarPart::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 36 (RenderScrollbarPart.cpp:120)
14  WebCore       0x10ef0fbe0 WebCore::RenderElement::setStyle(WebCore::RenderStyle&&, WebCore::StyleDifference) + 328 (RenderElement.cpp:503)
15  WebCore       0x10ef89314 WebCore::RenderLayerScrollableArea::updateResizerStyle() + 284 (RenderLayerScrollableArea.cpp:1742)
16  WebCore       0x10ef64328 WebCore::RenderLayerScrollableArea::updateAllScrollbarRelatedStyle() + 32 (RenderLayerScrollableArea.cpp:1760) [inlined]
17  WebCore       0x10ef64328 WebCore::RenderLayer::styleChanged(WebCore::StyleDifference, WebCore::RenderStyle const*) + 712 (RenderLayer.cpp:5312)
18  WebCore       0x10ef48e88 WebCore::RenderLayerModelObject::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 192 (RenderLayerModelObject.cpp:165)
19  WebCore       0x10eeb2710 WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 56 (RenderBox.cpp:317)
20  WebCore       0x10eeb21a0 WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 36 (RenderBlock.cpp:459)
21  WebCore       0x10eed6b98 WebCore::RenderBlockFlow::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) + 36 (RenderBlockFlow.cpp:2015)
22  WebCore       0x10ef0fbe0 WebCore::RenderElement::setStyle(WebCore::RenderStyle&&, WebCore::StyleDifference) + 328 (RenderElement.cpp:503)
23  WebCore       0x10f0c6008 WebCore::RenderTreeUpdater::updateElementRenderer(WebCore::Element&, WebCore::Style::ElementUpdate const&) + 168
24  WebCore       0x10f0c57dc WebCore::RenderTreeUpdater::updateRenderTree(WebCore::ContainerNode&) + 1208 (RenderTreeUpdater.cpp:187)
25  WebCore       0x10f0c511c WebCore::RenderTreeUpdater::commit(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >) + 172 (RenderTreeUpdater.cpp:114)
26  WebCore       0x10e525334 WebCore::Document::updateRenderTree(std::__1::unique_ptr<WebCore::Style::Update const, std::__1::default_delete<WebCore::Style::Update const> >) + 100 (Document.cpp:2034)
27  WebCore       0x10e52558c WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType) + 536 (Document.cpp:2135)
28  WebCore       0x10e525d88 WebCore::Document::updateStyleIfNeeded() + 232 (Document.cpp:2245)
29  WebCore       0x10eb27170 WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive() + 320 (FrameView.cpp:4804)
30  WebCore       0x10eb73c78 WebCore::Page::layoutIfNeeded() + 16 (Page.cpp:1562) [inlined]
31  WebCore       0x10eb73c78 WebCore::Page::updateRendering() + 220 (Page.cpp:1640)
32  WebKit        0x1056fbd68 WebKit::TiledCoreAnimationDrawingArea::updateRendering(WebKit::TiledCoreAnimationDrawingArea::UpdateRenderingType) + 64 (TiledCoreAnimationDrawingArea.mm:432)
33  CoreFoundation       0x1bdb1d1a4 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 36

Regression:

I am seeing this on WebKit canonical revision 255608@main
Both asan and non-asan builds crash
Comment 1 Abigail F 2022-10-19 14:31:08 PDT
rdar://101236223
Comment 2 Abigail F 2022-10-19 14:31:29 PDT
Created attachment 463100 [details]
repro_1017b.html
Comment 3 Darin Adler 2022-10-19 18:16:55 PDT
Looks like the null pointer is at LayoutIntegrationBoxTree.cpp:290 calling downcast. Seems like layoutBoxForRenderer returned a reference that was a dereferenced null pointer.
Comment 4 zalan 2022-10-19 19:42:38 PDT
it's most likely this deference in BoxTree::layoutBoxForRenderer
return *const_cast<RenderObject&>(renderer).layoutBox();

so for some reason we end up with a renderer without an associated layout box.
Comment 5 zalan 2022-10-20 16:54:40 PDT
Created attachment 463128 [details]
Patch
Comment 6 zalan 2022-10-20 20:06:17 PDT
Created attachment 463131 [details]
[fast-cq]Patch
Comment 7 EWS 2022-10-21 05:36:33 PDT
Committed 255821@main (57f8ae9715e9): <https://commits.webkit.org/255821@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 463131 [details].