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

Source/core/rendering/shapes/ShapeOutsideInfo.cpp

 /*
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above
  *    copyright notice, this list of conditions and the following
  *    disclaimer.
@@ skipping 79 matching lines @@
     if (imageResource.isAccessAllowed(document.securityOrigin()))
         return true;
 
     const KURL& url = imageResource.url();
     String urlString = url.isNull() ? "''" : url.elidedString();
     document.addConsoleMessage(SecurityMessageSource, ErrorMessageLevel, "Unsafe attempt to load URL " + urlString + ".");
 
     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());
     LayoutSize marginBoxSizeDelta(renderBox.marginLogicalWidth() + renderBox.borderAndPaddingLogicalWidth(), renderBox.marginLogicalHeight() + renderBox.borderAndPaddingLogicalHeight());
     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())
         return *shape;
 
     const RenderStyle& style = *m_renderer.style();
     ASSERT(m_renderer.containingBlock());
     const RenderStyle& containingBlockStyle = *m_renderer.containingBlock()->style();
 
     WritingMode writingMode = containingBlockStyle.writingMode();
     LayoutUnit maximumValue = m_renderer.containingBlock() ? m_renderer.containingBlock()->contentWidth() : LayoutUnit();
     float margin = floatValueForLength(m_renderer.style()->shapeMargin(), maximumValue.toFloat());
 
     float shapeImageThreshold = style.shapeImageThreshold();
     ASSERT(style.shapeOutside());
     const ShapeValue& shapeValue = *style.shapeOutside();
 
     switch (shapeValue.type()) {
     case ShapeValue::Shape:
         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);
         break;
     }
     }
 
     ASSERT(m_shape);
     return *m_shape;
 }
@@ skipping 179 matching lines @@
 }
 
 FloatSize ShapeOutsideInfo::shapeToRendererSize(FloatSize size) const
 {
     if (!m_renderer.style()->isHorizontalWritingMode())
         return size.transposedSize();
     return size;
 }
 
 }