Bug 220350 – null ptr deref with large background and -webkit-filter

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Ryosuke Niwa]

Created attachment 417068 [details]
Test

ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000754461963 bp 0x7ffeed8d3bb0 sp 0x7ffeed8d39c0 T0)

    #0 0x754461963 in WebCore::CSSFilterImageValue::image(WebCore::RenderElement&, WebCore::FloatSize const&)+0x413 (WebCore.framework/Versions/A/WebCore:x86_64+0x341b963)
    #1 0x7544e2c21 in WebCore::CSSImageGeneratorValue::image(WebCore::RenderElement&, WebCore::FloatSize const&)+0x71 (WebCore.framework/Versions/A/WebCore:x86_64+0x349cc21)
    #2 0x7568d655c in WebCore::StyleGeneratedImage::image(WebCore::RenderElement*, WebCore::FloatSize const&) const+0x2c (WebCore.framework/Versions/A/WebCore:x86_64+0x589055c)
    #3 0x7564f0956 in WebCore::RenderBoxModelObject::paintFillLayerExtended(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const&, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::InlineFlowBox*, WebCore::LayoutSize const&, WebCore::CompositeOperator, WebCore::RenderElement*, WebCore::BaseBackgroundColorUsage)+0x2896 (WebCore.framework/Versions/A/WebCore:x86_64+0x54aa956)
    #4 0x75648e6b3 in WebCore::RenderBox::paintFillLayer(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const&, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::CompositeOperator, WebCore::RenderElement*, WebCore::BaseBackgroundColorUsage)+0xf3 (WebCore.framework/Versions/A/WebCore:x86_64+0x54486b3)
    #5 0x75648afeb in WebCore::RenderBox::paintFillLayers(WebCore::PaintInfo const&, WebCore::Color const&, WebCore::FillLayer const&, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance, WebCore::CompositeOperator, WebCore::RenderElement*)+0x55b (WebCore.framework/Versions/A/WebCore:x86_64+0x5444feb)
    #6 0x75648a9d1 in WebCore::RenderBox::paintRootBoxFillLayers(WebCore::PaintInfo const&)+0x1e1 (WebCore.framework/Versions/A/WebCore:x86_64+0x54449d1)
    #7 0x75648bee7 in WebCore::RenderBox::paintBackground(WebCore::PaintInfo const&, WebCore::LayoutRect const&, WebCore::BackgroundBleedAvoidance)+0xd7 (WebCore.framework/Versions/A/WebCore:x86_64+0x5445ee7)
    #8 0x75648ba8a in WebCore::RenderBox::paintBoxDecorations(WebCore::PaintInfo&, WebCore::LayoutPoint const&)+0x41a (WebCore.framework/Versions/A/WebCore:x86_64+0x5445a8a)
    #9 0x756428667 in WebCore::RenderBlock::paintObject(WebCore::PaintInfo&, WebCore::LayoutPoint const&)+0x1a7 (WebCore.framework/Versions/A/WebCore:x86_64+0x53e2667)
    #10 0x756425dd5 in WebCore::RenderBlock::paint(WebCore::PaintInfo&, WebCore::LayoutPoint const&)+0x255 (WebCore.framework/Versions/A/WebCore:x86_64+0x53dfdd5)
    #11 0x75660e6d2 in WebCore::RenderLayer::paintBackgroundForFragments(WTF::Vector<WebCore::LayerFragment, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebCore::GraphicsContext&, WebCore::GraphicsContext&, WebCore::LayoutRect const&, bool, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::RenderObject*)+0x412 (WebCore.framework/Versions/A/WebCore:x86_64+0x55c86d2)
    #12 0x756607614 in WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0xdd4 (WebCore.framework/Versions/A/WebCore:x86_64+0x55c1614)
    #13 0x7566066fe in WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x23e (WebCore.framework/Versions/A/WebCore:x86_64+0x55c06fe)
    #14 0x756604422 in WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x452 (WebCore.framework/Versions/A/WebCore:x86_64+0x55be422)
    #15 0x756602e9f in WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x1ff (WebCore.framework/Versions/A/WebCore:x86_64+0x55bce9f)
    #16 0x75660e98b in WebCore::RenderLayer::paintList(WebCore::RenderLayer::LayerList, WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x13b (WebCore.framework/Versions/A/WebCore:x86_64+0x55c898b)
    #17 0x7566077d5 in WebCore::RenderLayer::paintLayerContents(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0xf95 (WebCore.framework/Versions/A/WebCore:x86_64+0x55c17d5)
    #18 0x7566066fe in WebCore::RenderLayer::paintLayerContentsAndReflection(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x23e (WebCore.framework/Versions/A/WebCore:x86_64+0x55c06fe)
    #19 0x756604422 in WebCore::RenderLayer::paintLayerWithEffects(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x452 (WebCore.framework/Versions/A/WebCore:x86_64+0x55be422)
    #20 0x756602e9f in WebCore::RenderLayer::paintLayer(WebCore::GraphicsContext&, WebCore::RenderLayer::LayerPaintingInfo const&, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>)+0x1ff (WebCore.framework/Versions/A/WebCore:x86_64+0x55bce9f)
    #21 0x756602a35 in WebCore::RenderLayer::paint(WebCore::GraphicsContext&, WebCore::LayoutRect const&, WebCore::LayoutSize const&, WTF::OptionSet<WebCore::PaintBehavior>, WebCore::RenderObject*, WTF::OptionSet<WebCore::RenderLayer::PaintLayerFlag>, WebCore::RenderLayer::SecurityOriginPaintPolicy, WebCore::EventRegionContext*)+0x2a5 (WebCore.framework/Versions/A/WebCore:x86_64+0x55bca35)
    #22 0x7559e55b3 in WebCore::FrameView::paintContents(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::Widget::SecurityOriginPaintPolicy, WebCore::EventRegionContext*)+0x573 (WebCore.framework/Versions/A/WebCore:x86_64+0x499f5b3)
    #23 0x755c8c546 in WebCore::ScrollView::paint(WebCore::GraphicsContext&, WebCore::IntRect const&, WebCore::Widget::SecurityOriginPaintPolicy, WebCore::EventRegionContext*)+0x526 (WebCore.framework/Versions/A/WebCore:x86_64+0x4c46546)
    #24 0x75637df21 in WebCore::ContentfulPaintChecker::qualifiesForContentfulPaint(WebCore::FrameView&)+0x201 (WebCore.framework/Versions/A/WebCore:x86_64+0x5337f21)
    #25 0x754818059 in WebCore::Document::enqueuePaintTimingEntryIfNeeded()+0x99 (WebCore.framework/Versions/A/WebCore:x86_64+0x37d2059)
    #26 0x755a99d88 in WebCore::Page::doAfterUpdateRendering()::$_28::operator()(WebCore::Document&) const+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x4a53d88)
    #27 0x755a99d63 in WTF::Detail::CallableWrapper<WebCore::Page::doAfterUpdateRendering()::$_28, void, WebCore::Document&>::call(WebCore::Document&)+0x13 (WebCore.framework/Versions/A/WebCore:x86_64+0x4a53d63)
    #28 0x755a61213 in WTF::Function<void (WebCore::Document&)>::operator()(WebCore::Document&) const+0x53 (WebCore.framework/Versions/A/WebCore:x86_64+0x4a1b213)
    #29 0x755a48f32 in WebCore::Page::forEachDocument(WTF::Function<void (WebCore::Document&)> const&) const+0x212 (WebCore.framework/Versions/A/WebCore:x86_64+0x4a02f32)
    #30 0x755a5535b in WebCore::Page::doAfterUpdateRendering()+0x1bb (WebCore.framework/Versions/A/WebCore:x86_64+0x4a0f35b)
    #31 0x755a54b40 in WebCore::Page::updateRendering()+0x7c0 (WebCore.framework/Versions/A/WebCore:x86_64+0x4a0eb40)
    #32 0x1047f8dd4 in WebKit::WebPage::updateRendering()+0x14 (WebKit.framework/Versions/A/WebKit:x86_64+0x24b6dd4)
    #33 0x103ff9e61 in WebKit::TiledCoreAnimationDrawingArea::updateRendering(WebKit::TiledCoreAnimationDrawingArea::UpdateRenderingType)+0x151 (WebKit.framework/Versions/A/WebKit:x86_64+0x1cb7e61)
    #34 0x104003065 in WebKit::TiledCoreAnimationDrawingArea::updateRenderingRunLoopCallback()+0x25 (WebKit.framework/Versions/A/WebKit:x86_64+0x1cc1065)
    #35 0x104017844 in WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(WebKit::WebPage&, WebKit::WebPageCreationParameters const&)::$_0::operator()() const+0x24 (WebKit.framework/Versions/A/WebKit:x86_64+0x1cd5844)
    #36 0x10401780c in WTF::Detail::CallableWrapper<WebKit::TiledCoreAnimationDrawingArea::TiledCoreAnimationDrawingArea(WebKit::WebPage&, WebKit::WebPageCreationParameters const&)::$_0, void>::call()+0xc (WebKit.framework/Versions/A/WebKit:x86_64+0x1cd580c)

<rdar://problem/72095621>

[Rob Buis]

This is easy to fix, will make a complete patch later:

--- a/Source/WebCore/css/CSSFilterImageValue.cpp
+++ b/Source/WebCore/css/CSSFilterImageValue.cpp
@@ -131,7 +131,10 @@ RefPtr<Image> CSSFilterImageValue::image(RenderElement& renderer, const FloatSiz
         return &Image::nullImage();
     cssFilter->apply();
 
-    return cssFilter->output()->copyImage();
+    if (auto* output = cssFilter->output())
+        return output->copyImage();
+
+    return &Image::nullImage();
 }

It does not seem like a security problem to me.

[Rob Buis]

Created attachment 417354 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 417354 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417354&action=review

> Source/WebCore/css/CSSFilterImageValue.cpp:137
> +    if (auto* output = cssFilter->output())
> +        return output->copyImage();
> +
> +    return &Image::nullImage();

Please flip the condition so that returning nullImage when output is null will be an early exist
and the normal flow of control when it's not null continues forward (i.e. output->copyImage() will be the last line of code).

[Rob Buis]

Created attachment 417437 [details]
Patch

[EWS]

Committed r271392: <https://trac.webkit.org/changeset/271392>

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