[CG] An image should not invoke many system calls before confirming its format is...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2017 20:38:21 +0000 (20:38 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2017 20:38:21 +0000 (20:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174692

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2017-07-21
Reviewed by Tim Horton.

We should be careful when invoking system calls before confirming that the
image type is available and it is one of the whitelist formats. Otherwise
we will be calling the parsers of the unsupported formats.

* loader/cache/CachedImage.cpp:
(WebCore::CachedImage::setImageDataBuffer): The check isAllowedImageUTI()
is now done in ImageDecoder::encodedDataStatus() which will return Error
if there is an error in the data or "isAllowedImageUTI() returns false."

* platform/graphics/BitmapImage.cpp:
(WebCore::BitmapImage::dataChanged): Avoid calling canUseAsyncDecodingForLargeImages()
before confirming the image type is available and it's supported by WebKit.
canUseAsyncDecodingForLargeImages() tries to cache the first frame of the
image to know its size. Asking the ImageFrameCache to destroy its decoded
frames is not needed unless ImageFrameCache::decodedSize() is not zero.

* platform/graphics/cg/ImageDecoderCG.cpp:
(WebCore::ImageDecoder::encodedDataStatus): Avoid calling CGImageSourceGetStatus()
before knowing the UTI of the image. When knowing it, we call CGImageSourceGetStatus()
and if it returns kCGImageStatusIncomplete or kCGImageStatusComplete, we
check whether isAllowedImageUTI() or not. If isAllowedImageUTI() returns
false, return Error which will make the CachedImage cancel loading the
rest of the image.

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

Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedImage.cpp
Source/WebCore/platform/graphics/BitmapImage.cpp
Source/WebCore/platform/graphics/cg/ImageDecoderCG.cpp

index 2147389..a53efaf 100644 (file)
@@ -1,3 +1,34 @@
+2017-07-21  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        [CG] An image should not invoke many system calls before confirming its format is supported
+        https://bugs.webkit.org/show_bug.cgi?id=174692
+
+        Reviewed by Tim Horton.
+
+        We should be careful when invoking system calls before confirming that the
+        image type is available and it is one of the whitelist formats. Otherwise
+        we will be calling the parsers of the unsupported formats.
+
+        * loader/cache/CachedImage.cpp:
+        (WebCore::CachedImage::setImageDataBuffer): The check isAllowedImageUTI()
+        is now done in ImageDecoder::encodedDataStatus() which will return Error
+        if there is an error in the data or "isAllowedImageUTI() returns false."
+
+        * platform/graphics/BitmapImage.cpp:
+        (WebCore::BitmapImage::dataChanged): Avoid calling canUseAsyncDecodingForLargeImages()
+        before confirming the image type is available and it's supported by WebKit.
+        canUseAsyncDecodingForLargeImages() tries to cache the first frame of the
+        image to know its size. Asking the ImageFrameCache to destroy its decoded
+        frames is not needed unless ImageFrameCache::decodedSize() is not zero.
+
+        * platform/graphics/cg/ImageDecoderCG.cpp:
+        (WebCore::ImageDecoder::encodedDataStatus): Avoid calling CGImageSourceGetStatus()
+        before knowing the UTI of the image. When knowing it, we call CGImageSourceGetStatus()
+        and if it returns kCGImageStatusIncomplete or kCGImageStatusComplete, we
+        check whether isAllowedImageUTI() or not. If isAllowedImageUTI() returns
+        false, return Error which will make the CachedImage cancel loading the 
+        rest of the image.
+
 2017-07-21  Jeremy Jones  <jeremyj@apple.com>
 
         AudioTrackPrivateMediaStreamCocoa shouldn't set AudioSession::setPreferredBufferSize
index a000ec0..97026e6 100644 (file)
@@ -51,7 +51,6 @@
 
 #if USE(CG)
 #include "PDFDocumentImage.h"
-#include "UTIRegistry.h"
 #endif
 
 namespace WebCore {
@@ -440,12 +439,7 @@ void CachedImage::addIncrementalDataBuffer(SharedBuffer& data)
 
 EncodedDataStatus CachedImage::setImageDataBuffer(SharedBuffer* data, bool allDataReceived)
 {
-    EncodedDataStatus encodedDataStatus = m_image ? m_image->setData(data, allDataReceived) : EncodedDataStatus::Error;
-#if USE(CG)
-    if (encodedDataStatus >= EncodedDataStatus::TypeAvailable && m_image->isBitmapImage() && !isAllowedImageUTI(m_image->uti()))
-        return EncodedDataStatus::Error;
-#endif
-    return encodedDataStatus;
+    return m_image ? m_image->setData(data, allDataReceived) : EncodedDataStatus::Error;
 }
 
 void CachedImage::addDataBuffer(SharedBuffer& data)
index 0b98006..dd5fc8c 100644 (file)
@@ -107,7 +107,7 @@ void BitmapImage::destroyDecodedDataIfNecessary(bool destroyAll)
 
 EncodedDataStatus BitmapImage::dataChanged(bool allDataReceived)
 {
-    if (!canUseAsyncDecodingForLargeImages())
+    if (m_source.decodedSize() && !canUseAsyncDecodingForLargeImages())
         m_source.destroyIncompleteDecodedData();
 
     m_currentFrameDecodingStatus = ImageFrame::DecodingStatus::Invalid;
index 85254ad..767441e 100644 (file)
@@ -35,6 +35,7 @@
 #include "IntSize.h"
 #include "Logging.h"
 #include "SharedBuffer.h"
+#include "UTIRegistry.h"
 
 #if !PLATFORM(IOS)
 #include <ApplicationServices/ApplicationServices.h>
@@ -184,6 +185,10 @@ String ImageDecoder::filenameExtension() const
 
 EncodedDataStatus ImageDecoder::encodedDataStatus() const
 {
+    String uti = this->uti();
+    if (uti.isEmpty())
+        return EncodedDataStatus::Unknown;
+
     switch (CGImageSourceGetStatus(m_nativeDecoder.get())) {
     case kCGImageStatusUnknownType:
         return EncodedDataStatus::Error;
@@ -198,6 +203,9 @@ EncodedDataStatus ImageDecoder::encodedDataStatus() const
         return EncodedDataStatus::Error;
 
     case kCGImageStatusIncomplete: {
+        if (!isAllowedImageUTI(uti))
+            return EncodedDataStatus::Error;
+
         RetainPtr<CFDictionaryRef> image0Properties = adoptCF(CGImageSourceCopyPropertiesAtIndex(m_nativeDecoder.get(), 0, imageSourceOptions().get()));
         if (!image0Properties)
             return EncodedDataStatus::TypeAvailable;
@@ -209,6 +217,9 @@ EncodedDataStatus ImageDecoder::encodedDataStatus() const
     }
 
     case kCGImageStatusComplete:
+        if (!isAllowedImageUTI(uti))
+            return EncodedDataStatus::Error;
+
         return EncodedDataStatus::Complete;
     }