[Chromium] SVG Composite of Offset crashes
authorschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 01:00:17 +0000 (01:00 +0000)
committerschenney@chromium.org <schenney@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 01:00:17 +0000 (01:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77245

Reviewed by Stephen White.

The feComposite arithmetic mode filter could readily be made to
generate invalid pre-multiplied pixel values which would then go on to
pollute other filters and cause invalid final output pixels. This
patch checks for filters that require valid inputs, and checks that a
result is valid, and corrects the result if necessary. This matches
the behavior of FF and Opera while preventing crashes or other
undesirable behavior.

Source/WebCore:

Test: svg/filters/feComposite-arithmetic-invalid-rgba.svg

* platform/graphics/filters/FEComposite.h: Override the default validity checks and image cleanup methods.
* platform/graphics/filters/FEComposite.cpp:
(WebCore::FEComposite::correctFilterResultIfNeeded): Force valid pixels if this is an arithmetic filter
* platform/graphics/filters/FilterEffect.cpp:
(WebCore::FilterEffect::apply): Check for validity status and correct
(WebCore::FilterEffect::forceValidPremultipliedPixels): Make an image valid
(WebCore):
* platform/graphics/filters/FilterEffect.h: New virtual methods for image validity.
(FilterEffect):
(WebCore::FilterEffect::requiresValidPreMulultipliedPixels):
(WebCore::FilterEffect::forceValidPremultipliedPixels):
(WebCore::FilterEffect::correctFilterResultIfNeeded):
* rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::postApplyResource): Check that the final filter result is valid

LayoutTests:

* svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg: Added.
* svg/filters/feComposite-arithmetic-invalid-rgba.svg: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109820 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg [new file with mode: 0644]
LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/filters/FEComposite.cpp
Source/WebCore/platform/graphics/filters/FEComposite.h
Source/WebCore/platform/graphics/filters/FilterEffect.cpp
Source/WebCore/platform/graphics/filters/FilterEffect.h
Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp

index 250fe4ce962aecdc161c6263105f8dccf3fbf827..1e9078e56fd577bf6124fda926393a4747186747 100644 (file)
@@ -1,3 +1,21 @@
+2012-03-05  Stephen Chenney  <schenney@chromium.org>
+
+        [Chromium] SVG Composite of Offset crashes
+        https://bugs.webkit.org/show_bug.cgi?id=77245
+
+        Reviewed by Stephen White.
+
+        The feComposite arithmetic mode filter could readily be made to
+        generate invalid pre-multiplied pixel values which would then go on to
+        pollute other filters and cause invalid final output pixels. This
+        patch checks for filters that require valid inputs, and checks that a
+        result is valid, and corrects the result if necessary. This matches
+        the behavior of FF and Opera while preventing crashes or other
+        undesirable behavior.
+
+        * svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg: Added.
+        * svg/filters/feComposite-arithmetic-invalid-rgba.svg: Added.
+
 2012-03-05  Alexis Menard  <alexis.menard@openbossa.org>
 
         getComputedStyle gives incorrect information for 'height' property
diff --git a/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg b/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba-expected.svg
new file mode 100644 (file)
index 0000000..bf3babc
--- /dev/null
@@ -0,0 +1,18 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="100">
+    <!-- Due to Bug 80321, we need to use a filter in this expected result reference. -->
+    <defs>
+        <!-- This filter produces the same as the input, but does color conversions along the way. -->
+        <filter id="identity">
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="SourceGraphic" k1="0" k2="1.0" k3="0" k4="0" />
+        </filter>
+    </defs>
+
+    <!-- Background for color comparison. The border of the final rectangle should be the same as the interior color. -->
+    <rect x="20" y="20" width="60" height="60" fill="rgba(0,255,0,1)" />
+
+    <!-- The content of interest -->
+    <rect filter="url(#identity)" x="25" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+
+    <!-- Border to show expected non-drawing area. -->
+    <rect x="125" y="25" width="50" height="50" fill="none" stroke="rgb(0,255,0)" />
+</svg>
diff --git a/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg b/LayoutTests/svg/filters/feComposite-arithmetic-invalid-rgba.svg
new file mode 100644 (file)
index 0000000..c445d93
--- /dev/null
@@ -0,0 +1,33 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="200" height="100">
+    <defs>
+        <!-- This filter produces results that are invalid pre-multiplied rgba pixels. Specifically, after the 4th step an -->
+        <!-- interior pixel will contain approximately (0, 0.8, 0, 0.6) which is invalid because g > a. When used in     -->
+        <!-- other operations this may generate bad results but we only want to clamp the values when passed on to other   -->
+        <!-- operations, not for intermediate arithmetic results. -->
+        <filter id="arithmetic">
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="SourceGraphic" k1="0" k2="0.2" k3="0" k4="0" result="rgba02" />
+            <feComposite operator="arithmetic" in="SourceAlpha" in2="SourceAlpha" k1="0" k2="0.2" k3="0" k4="0" result="alpha05" />
+            <feComposite operator="arithmetic" in="rgba02" in2="alpha05" k1="0" k2="1" k3="1" k4="0" result="tmp" />
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="tmp" k1="0" k2="1" k3="-1" k4="0" />
+            <feComposite operator="arithmetic" in="tmp" k1="0" k2="1" k3="1" k4="0" />
+        </filter>
+
+        <!-- This filter will produce images with zero alpha but non-zero color components. -->
+        <filter id="zero-alpha">
+            <feComposite operator="arithmetic" in="SourceAlpha" in2="SourceAlpha" k1="0" k2="1.0" k3="0" k4="0" result="alpha" />
+            <feComposite operator="arithmetic" in="SourceGraphic" in2="alpha" k1="0" k2="1.0" k3="-1.0" k4="0" />
+        </filter>
+    </defs>
+
+    <!-- Background for color comparison. The border of the final rectangle should be the same as the interior color. -->
+    <rect x="20" y="20" width="60" height="60" fill="rgba(0,255,0,1)" />
+
+    <!-- The content of interest -->
+    <rect filter="url(#arithmetic)" x="25" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+
+    <!-- Border to show expected non-drawing area. -->
+    <rect x="125" y="25" width="50" height="50" fill="none" stroke="rgb(0,255,0)" />
+
+    <!-- This should produce nothing, and not cause a crash. -->
+    <rect filter="url(#zero-alpha)" x="125" y="25" width="50" height="50" fill="rgba(0,255,0,1)" stroke="none" />
+</svg>
index 04570ec4752717d988f77dc1258c0b0234913902..c081d4fcdb63ebaa07081170f4ee0c4af9e3a3b8 100644 (file)
@@ -1,3 +1,35 @@
+2012-03-05  Stephen Chenney  <schenney@chromium.org>
+
+        [Chromium] SVG Composite of Offset crashes
+        https://bugs.webkit.org/show_bug.cgi?id=77245
+
+        Reviewed by Stephen White.
+
+        The feComposite arithmetic mode filter could readily be made to
+        generate invalid pre-multiplied pixel values which would then go on to
+        pollute other filters and cause invalid final output pixels. This
+        patch checks for filters that require valid inputs, and checks that a
+        result is valid, and corrects the result if necessary. This matches
+        the behavior of FF and Opera while preventing crashes or other
+        undesirable behavior.
+
+        Test: svg/filters/feComposite-arithmetic-invalid-rgba.svg
+
+        * platform/graphics/filters/FEComposite.h: Override the default validity checks and image cleanup methods.
+        * platform/graphics/filters/FEComposite.cpp:
+        (WebCore::FEComposite::correctFilterResultIfNeeded): Force valid pixels if this is an arithmetic filter
+        * platform/graphics/filters/FilterEffect.cpp:
+        (WebCore::FilterEffect::apply): Check for validity status and correct
+        (WebCore::FilterEffect::forceValidPremultipliedPixels): Make an image valid
+        (WebCore):
+        * platform/graphics/filters/FilterEffect.h: New virtual methods for image validity.
+        (FilterEffect):
+        (WebCore::FilterEffect::requiresValidPreMulultipliedPixels):
+        (WebCore::FilterEffect::forceValidPremultipliedPixels):
+        (WebCore::FilterEffect::correctFilterResultIfNeeded):
+        * rendering/svg/RenderSVGResourceFilter.cpp:
+        (WebCore::RenderSVGResourceFilter::postApplyResource): Check that the final filter result is valid
+
 2012-03-05  Alexis Menard  <alexis.menard@openbossa.org>
 
         getComputedStyle gives incorrect information for 'height' property
index fbc2c1671f88f1922bc9430ceacf36b17085b8df..5be22f8e29deba584d24ba1f2dcce8798945abfc 100644 (file)
@@ -116,6 +116,14 @@ bool FEComposite::setK4(float k4)
     return true;
 }
 
+void FEComposite::correctFilterResultIfNeeded()
+{
+    if (m_type != FECOMPOSITE_OPERATOR_ARITHMETIC)
+        return;
+
+    forceValidPreMultipliedPixels();
+}
+
 template <int b1, int b2, int b3, int b4>
 static inline void computeArithmeticPixels(unsigned char* source, unsigned char* destination, int pixelArrayLength,
                                     float k1, float k2, float k3, float k4)
index 35f2505e386468ff8d94bd056e5ec480b499e462..24ba3078f1e3bd588c599a31804874b71e66a3e7 100644 (file)
@@ -59,6 +59,8 @@ public:
     float k4() const;
     bool setK4(float);
 
+    virtual void correctFilterResultIfNeeded() OVERRIDE;
+
     virtual void platformApplySoftware();
     virtual void dump();
     
@@ -66,6 +68,9 @@ public:
 
     virtual TextStream& externalRepresentation(TextStream&, int indention) const;
 
+protected:
+    virtual bool requiresValidPreMultipliedPixels() OVERRIDE { return m_type != FECOMPOSITE_OPERATOR_ARITHMETIC; }
+
 private:
     FEComposite(Filter*, const CompositeOperationType&, float, float, float, float);
 
index 37eb8c71571a7604572a94880929e4a051a87b37..9bf1689f7fc84a8398dd46b439de0c7c8171d5d9 100644 (file)
@@ -102,6 +102,11 @@ void FilterEffect::apply()
             return;
     }
     determineAbsolutePaintRect();
+
+    if (requiresValidPreMultipliedPixels()) {
+        for (unsigned i = 0; i < size; ++i)
+            inputEffect(i)->correctFilterResultIfNeeded();
+    }
     
     // Add platform specific apply functions here and return earlier.
 #if USE(SKIA)
@@ -111,6 +116,35 @@ void FilterEffect::apply()
     platformApplySoftware();
 }
 
+void FilterEffect::forceValidPreMultipliedPixels()
+{
+    // Must operate on pre-multiplied results; other formats cannot have invalid pixels.
+    if (!m_premultipliedImageResult)
+        return;
+
+    ByteArray* imageArray = m_premultipliedImageResult.get();
+    unsigned char* pixelData = imageArray->data();
+    int pixelArrayLength = imageArray->length();
+
+    // We must have four bytes per pixel, and complete pixels
+    ASSERT(!(pixelArrayLength % 4));
+    int numPixels = pixelArrayLength / 4;
+
+    // Iterate over each pixel, checking alpha and adjusting color components if necessary
+    while (--numPixels >= 0) {
+        // Alpha is the 4th byte in a pixel
+        unsigned char a = *(pixelData + 3);
+        // Clamp each component to alpha, and increment the pixel location
+        for (int i = 0; i < 3; ++i) {
+            if (*pixelData > a)
+                *pixelData = a;
+            ++pixelData;
+        }
+        // Increment for alpha
+        ++pixelData;
+    }
+}
+
 void FilterEffect::clearResult()
 {
     if (m_imageBufferResult)
index 1bae4ca0ad7b99668627cea14b172c81e81f773f..8b4ad6f922752fde21ba85dcb1306d3a8eae2c69 100644 (file)
@@ -86,6 +86,11 @@ public:
 
     void apply();
     
+    // Correct any invalid pixels, if necessary, in the result of a filter operation.
+    // This method is used to ensure valid pixel values on filter inputs and the final result.
+    // Only the arithmetic composite filter ever needs to perform correction.
+    virtual void correctFilterResultIfNeeded() { }
+
     virtual void platformApplySoftware() = 0;
 #if USE(SKIA)
     virtual bool platformApplySkia() { return false; }
@@ -131,6 +136,13 @@ protected:
     ByteArray* createUnmultipliedImageResult();
     ByteArray* createPremultipliedImageResult();
 
+    // Return true if the filter will only operate correctly on valid RGBA values, with
+    // alpha in [0,255] and each color component in [0, alpha].
+    virtual bool requiresValidPreMultipliedPixels() { return true; }
+
+    // If a pre-multiplied image, check every pixel for validity and correct if necessary.
+    void forceValidPreMultipliedPixels();
+
 private:
     OwnPtr<ImageBuffer> m_imageBufferResult;
     RefPtr<ByteArray> m_unmultipliedImageResult;
index cf4303d1380edc2d91936bd297a118dfc76d27c9..43ebee284d0687312cfbe0d6ba20246bbd26dac8 100644 (file)
@@ -300,6 +300,7 @@ void RenderSVGResourceFilter::postApplyResource(RenderObject* object, GraphicsCo
         // Always true if filterData is just built (filterData->builded is false).
         if (!lastEffect->hasResult()) {
             lastEffect->apply();
+            lastEffect->correctFilterResultIfNeeded();
 #if !USE(CG)
             ImageBuffer* resultImage = lastEffect->asImageBuffer();
             if (resultImage)