[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 21473890d63564ade9b7fc900f127e6c210259fb..a53efafd3f94a1c9d7cc009dee1405256b02960f 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 a000ec0b33767ce34b07d3411dc413ca68240bd7..97026e6f457f4b3bc61bf01955d3003b42dfae94 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 0b98006467c0cc8eae8a0e2a9b6e994d2db8e085..dd5fc8ce395c288cac3b840b5419cd3d0995b87d 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 85254ad5e9bc3ddf45eb07d2bf8207b32df6714b..767441e0a2b7767c8146b7b084ff801546643561 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;
     }