Heap-use-after-free in WebCore::GraphicsContext::drawImage

Source/core/rendering/shapes/ShapeOutsideInfo.cpp

Index: Source/core/rendering/shapes/ShapeOutsideInfo.cpp
diff --git a/Source/core/rendering/shapes/ShapeOutsideInfo.cpp b/Source/core/rendering/shapes/ShapeOutsideInfo.cpp
index fabf6fa86de9540cd1a5ff29f714b507fe433417..baf91b0d98a39af210c28ffe2e19f721d489bf11 100644
--- a/Source/core/rendering/shapes/ShapeOutsideInfo.cpp
+++ b/Source/core/rendering/shapes/ShapeOutsideInfo.cpp
@@ -97,26 +97,6 @@ static bool checkShapeImageOrigin(Document& document, const StyleImage& styleIma
     return false;
 }
 
-static void getShapeImageAndRect(const ShapeValue& shapeValue, const RenderBox& renderBox, const LayoutSize& referenceBoxSize, Image*& image, LayoutRect& rect)
-{
-    ASSERT(shapeValue.isImageValid());
-    StyleImage* styleImage = shapeValue.image();
-
-    const IntSize& imageSize = renderBox.calculateImageIntrinsicDimensions(styleImage, roundedIntSize(referenceBoxSize), RenderImage::ScaleByEffectiveZoom);
-    styleImage->setContainerSizeForRenderer(&renderBox, imageSize, renderBox.style()->effectiveZoom());
-
-    image = 0;
-    if (styleImage->isImageResource() || styleImage->isImageResourceSet())
-        image = styleImage->cachedImage()->imageForRenderer(&renderBox);
-    else if (styleImage->isGeneratedImage())
-        image = styleImage->image(const_cast<RenderBox*>(&renderBox), imageSize).get();
-
-    if (renderBox.isRenderImage())
-        rect = toRenderImage(&renderBox)->replacedContentRect();
-    else
-        rect = LayoutRect(LayoutPoint(), imageSize);
-}
-
 static LayoutRect getShapeImageMarginRect(const RenderBox& renderBox, const LayoutSize& referenceBoxLogicalSize)
 {
     LayoutPoint marginBoxOrigin(-renderBox.marginLogicalLeft() - renderBox.borderAndPaddingLogicalLeft(), -renderBox.marginBefore() - renderBox.borderBefore() - renderBox.paddingBefore());
@@ -124,6 +104,29 @@ static LayoutRect getShapeImageMarginRect(const RenderBox& renderBox, const Layo
     return LayoutRect(marginBoxOrigin, referenceBoxLogicalSize + marginBoxSizeDelta);
 }
 
+PassOwnPtr<Shape> ShapeOutsideInfo::createShapeForImage(StyleImage* styleImage, float shapeImageThreshold, WritingMode writingMode, float margin) const
+{
+    const IntSize& imageSize = m_renderer.calculateImageIntrinsicDimensions(styleImage, roundedIntSize(m_referenceBoxLogicalSize), RenderImage::ScaleByEffectiveZoom);
+    styleImage->setContainerSizeForRenderer(&m_renderer, imageSize, m_renderer.style()->effectiveZoom());
+
+    const LayoutRect& marginRect = getShapeImageMarginRect(m_renderer, m_referenceBoxLogicalSize);
+    const LayoutRect& imageRect = (m_renderer.isRenderImage())
+        ? toRenderImage(&m_renderer)->replacedContentRect()
+        : LayoutRect(LayoutPoint(), imageSize);
+
+    Image* image = 0;

[eseidel, 2014/05/30 21:06:45]

Might as well just make this a RefPtr and then call .release() when passing it
off to createRasterShape?

+    RefPtr<Image> generatedImage;

[rwlbuis, 2014/05/30 21:14:05]

Nit: could move this into the else if section since that is the only scope it is
used in.

+
+    if (styleImage->isImageResource() || styleImage->isImageResourceSet()) {
+        image = styleImage->cachedImage()->imageForRenderer(&m_renderer);
+    } else if (styleImage->isGeneratedImage()) {
+        generatedImage = styleImage->image(const_cast<RenderBox*>(&m_renderer), imageSize).get();
+        image = generatedImage.get();

[eseidel, 2014/05/30 21:22:08]

I was more reacting to this line which is strange.  This creates a raw pointer
to generatedImage which is RefCounted and who knows when it might go away.  If
image is a RefPtr then this looks like:

image = generatedImage;

and then the later line is:

createRasterShape(image.release(), ...)

And then there is no question about some future refactoring causing
generatedImage to be dead before the raw ptr image is cleared, because image is
no longer a raw ptr and keeps its own ref.

+    }
+
+    return Shape::createRasterShape(image, shapeImageThreshold, imageRect, marginRect, writingMode, margin);
+}
+
 const Shape& ShapeOutsideInfo::computedShape() const
 {
     if (Shape* shape = m_shape.get())
@@ -146,14 +149,10 @@ const Shape& ShapeOutsideInfo::computedShape() const
         ASSERT(shapeValue.shape());
         m_shape = Shape::createShape(shapeValue.shape(), m_referenceBoxLogicalSize, writingMode, margin);
         break;
-    case ShapeValue::Image: {
-        Image* image;
-        LayoutRect imageRect;
-        getShapeImageAndRect(shapeValue, m_renderer, m_referenceBoxLogicalSize, image, imageRect);
-        const LayoutRect& marginRect = getShapeImageMarginRect(m_renderer, m_referenceBoxLogicalSize);
-        m_shape = Shape::createRasterShape(image, shapeImageThreshold, imageRect, marginRect, writingMode, margin);
+    case ShapeValue::Image:
+        ASSERT(shapeValue.isImageValid());
+        m_shape = createShapeForImage(shapeValue.image(), shapeImageThreshold, writingMode, margin);
         break;
-    }
     case ShapeValue::Box: {
         const RoundedRect& shapeRect = style.getRoundedBorderFor(LayoutRect(LayoutPoint(), m_referenceBoxLogicalSize), m_renderer.view());
         m_shape = Shape::createLayoutBoxShape(shapeRect, writingMode, margin);