Bugfix: PNG/JPEG workaround for GDI+ bitmaps > INT_MAX in size

Merge PR #315 by https://github.com/qbnu
sylikc · Jul 6, 2024 · a78d436 · a78d436
2 parents 69bdf62 + 0ec60fa
commit a78d436
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 11 deletions.
diff --git a/src/JPEGView/ImageLoadThread.cpp b/src/JPEGView/ImageLoadThread.cpp
@@ -190,14 +190,17 @@ static CJPEGImage* ConvertGDIPlusBitmapToJPEGImage(Gdiplus::Bitmap* pBitmap, int
 
 	Gdiplus::Rect bmRect(0, 0, pBitmap->GetWidth(), pBitmap->GetHeight());
 	Gdiplus::BitmapData bmData;
-	if (pBitmapToUse->LockBits(&bmRect, Gdiplus::ImageLockModeRead, PixelFormat32bppRGB, &bmData) == Gdiplus::Ok) {
+	lastStatus = pBitmapToUse->LockBits(&bmRect, Gdiplus::ImageLockModeRead, PixelFormat32bppRGB, &bmData);
+	if (lastStatus == Gdiplus::Ok) {
 		assert(bmData.PixelFormat == PixelFormat32bppRGB);
 		void* pDIB = CBasicProcessing::ConvertGdiplus32bppRGB(bmRect.Width, bmRect.Height, bmData.Stride, bmData.Scan0);
 		if (pDIB != NULL) {
 			pJPEGImage = new CJPEGImage(bmRect.Width, bmRect.Height, pDIB, pEXIFData, 4, nJPEGHash, eImageFormat,
 				eImageFormat == IF_GIF && nFrameCount > 1, nFrameIndex, nFrameCount, nFrameTimeMs);
 		}
 		pBitmapToUse->UnlockBits(&bmData);
+	} else if (lastStatus == Gdiplus::ValueOverflow) {
+		isOutOfMemory = true;
 	}
 
 	if (pBmGraphics != NULL && pBmTarget != NULL) {
@@ -494,7 +497,8 @@ void CImageLoadThread::ProcessReadJPEGRequest(CRequest * request) {
 		}
 		unsigned int nNumBytesRead;
 		if (::ReadFile(hFile, pBuffer, nFileSize, (LPDWORD) &nNumBytesRead, NULL) && nNumBytesRead == nFileSize) {
-			if (CSettingsProvider::This().ForceGDIPlus() || CSettingsProvider::This().UseEmbeddedColorProfiles()) {
+			bool bUseGDIPlus = CSettingsProvider::This().ForceGDIPlus() || CSettingsProvider::This().UseEmbeddedColorProfiles();
+			if (bUseGDIPlus) {
 				IStream* pStream = NULL;
 				if (::CreateStreamOnHGlobal(hFileBuffer, FALSE, &pStream) == S_OK) {
 					Gdiplus::Bitmap* pBitmap = Gdiplus::Bitmap::FromStream(pStream, CSettingsProvider::This().UseEmbeddedColorProfiles());
@@ -510,7 +514,8 @@ void CImageLoadThread::ProcessReadJPEGRequest(CRequest * request) {
 				} else {
 					request->OutOfMemory = true;
 				}
-			} else {
+			}
+			if (!bUseGDIPlus || request->OutOfMemory) {
 				int nWidth, nHeight, nBPP;
 				TJSAMP eChromoSubSampling;
 				bool bOutOfMemory;
@@ -695,7 +700,7 @@ void CImageLoadThread::ProcessReadPNGRequest(CRequest* request) {
 #ifndef WINXP
 			// If UseEmbeddedColorProfiles is true and the image isn't animated, we should use GDI+ for better color management
 			bool bUseGDIPlus = CSettingsProvider::This().ForceGDIPlus() || CSettingsProvider::This().UseEmbeddedColorProfiles();
-			if (bUseCachedDecoder || !bUseGDIPlus || PngReader::IsAnimated(pBuffer, nFileSize))
+			if (bUseCachedDecoder || !bUseGDIPlus || PngReader::MustUseLibpng(pBuffer, nFileSize))
 				pPixelData = (uint8*)PngReader::ReadImage(nWidth, nHeight, nBPP, bHasAnimation, nFrameCount, nFrameTimeMs, pEXIFData, request->OutOfMemory, pBuffer, nFileSize);
 #endif
 

diff --git a/src/JPEGView/PNGWrapper.cpp b/src/JPEGView/PNGWrapper.cpp
@@ -379,16 +379,28 @@ void PngReader::DeleteCache() {
 	DeleteCacheInternal(true);
 }
 
-bool PngReader::IsAnimated(void* buffer, size_t sizebytes) {
-	// Valid APNGs must have an acTL chunk before the first IDAT chunk, this lets us quickly determine if a PNG is animated
+bool PngReader::MustUseLibpng(const void* bufferIn, size_t sizebytes) {
+	const char* buffer = (const char*)bufferIn;
+
+	// GDI+ fails if the uncompressed image size is over INT_MAX
+	if (sizebytes < 24) {
+		return false;
+	}
+	unsigned int width = _byteswap_ulong(*(unsigned int*)(buffer + 16));
+	unsigned int height = _byteswap_ulong(*(unsigned int*)(buffer + 20));
+	if (4.0 * width * height > INT_MAX) {
+		return true;
+	}
 
+	// GDI+ does not support APNG
+	// Valid APNGs must have an acTL chunk before the first IDAT chunk, this lets us quickly determine if a PNG is animated
 	size_t offset = 8; // skip PNG signature
 	while (offset + 7 < sizebytes) {
-		if (memcmp((char*)buffer + offset + 4, "acTL", 4) == 0)
+		if (memcmp(buffer + offset + 4, "acTL", 4) == 0)
 			return true;
-		if (memcmp((char*)buffer + offset + 4, "IDAT", 4) == 0)
+		if (memcmp(buffer + offset + 4, "IDAT", 4) == 0)
 			return false;
-		unsigned int chunksize = *(unsigned int*)((char*)buffer + offset);
+		unsigned int chunksize = *(unsigned int*)(buffer + offset);
 
 		// PNG chunk sizes are big-endian and must be converted to little-endian
 		chunksize = _byteswap_ulong(chunksize);

diff --git a/src/JPEGView/PNGWrapper.h b/src/JPEGView/PNGWrapper.h
@@ -19,8 +19,8 @@ class PngReader
 
 	static void DeleteCache();
 
-	// Returns true if PNG is animated, false otherwise
-	static bool IsAnimated(void* buffer, size_t sizebytes);
+	// Returns true if PNG is unsupported by GDI+
+	static bool MustUseLibpng(const void* buffer, size_t sizebytes);
 #endif
 	// Get EXIF Block
 	static void* GetEXIFBlock(void* buffer, size_t sizebytes);