From: Indrek Altpere <efbiaiinzinz@hotmail.com>
Subject: winemenubuilder: fix crash caused by invalid icon entries and avoid future crashes by ignoring unhandled png entries
Message-Id: <DUB404-EAS14998152908C3C637831F6FCFDC0@phx.gbl>
Date: Tue, 26 Aug 2014 09:45:44 +0300

For the InnoSetup 5 crash (and likely other similar crashes), the issue
seems to be GRPICONDIRENTRY with invalid information.
The dwBytesInRes has a value that exceeds the Size value in
IMAGE_RESOURCE_DATA_ENTRY, causing out-of-bounds memcpy and thus crash.
Added check+clipping against the out-of-bounds read which fixes the
particular crash.

As per MSDN blog, icon resources can contain raw PNG information instead of
regular BITMAPINFO, but due to weird decisions, only way to differentiate
between them is to check if the resource starts with PNG header bytes.
http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
Made the winemenubuilder ignore such entries, since current winemenubuilder
logic only works with correct BITMAPINFO data (which raw PNG data is not).

Regards,
Indrek

diff --git a/programs/winemenubuilder/winemenubuilder.c b/programs/winemenubuilder/winemenubuilder.c
index 4675369..eac4cd6 100644
--- a/programs/winemenubuilder/winemenubuilder.c
+++ b/programs/winemenubuilder/winemenubuilder.c
@@ -599,6 +599,7 @@ static int populate_module_icons(HMODULE hModule, GRPICONDIR *grpIconDir, ICONDI
 {
     int i;
     int validEntries = 0;
+    static const BYTE png_header[] = { 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A };
 
     for (i = 0; i < grpIconDir->idCount; i++)
     {
@@ -610,19 +611,32 @@ static int populate_module_icons(HMODULE hModule, GRPICONDIR *grpIconDir, ICONDI
             if ((hResData = LoadResource(hModule, hResInfo)))
             {
                 BITMAPINFO *pIcon;
+                DWORD availableBytes = ((IMAGE_RESOURCE_DATA_ENTRY *)hResInfo)->Size;
+                DWORD iconBytes = grpIconDir->idEntries[i].dwBytesInRes;
+                if (iconBytes > availableBytes)
+                {
+                    WINE_ERR("Broken GRPICONENTRY tries to use %d bytes when only %d available, clipping\n", iconBytes, availableBytes);
+                    iconBytes = availableBytes;
+                }
                 if ((pIcon = LockResource(hResData)))
                 {
+                    BOOL isPNG = iconBytes >= sizeof(png_header) && memcmp(pIcon, png_header, sizeof(png_header)) == 0;
+                    if (isPNG)
+                    {
+                        WINE_FIXME("PNG icon not supported\n");
+                        continue;
+                    }
                     iconDirEntries[validEntries].bWidth = grpIconDir->idEntries[i].bWidth;
                     iconDirEntries[validEntries].bHeight = grpIconDir->idEntries[i].bHeight;
                     iconDirEntries[validEntries].bColorCount = grpIconDir->idEntries[i].bColorCount;
                     iconDirEntries[validEntries].bReserved = grpIconDir->idEntries[i].bReserved;
                     iconDirEntries[validEntries].wPlanes = grpIconDir->idEntries[i].wPlanes;
                     iconDirEntries[validEntries].wBitCount = grpIconDir->idEntries[i].wBitCount;
-                    iconDirEntries[validEntries].dwBytesInRes = grpIconDir->idEntries[i].dwBytesInRes;
+                    iconDirEntries[validEntries].dwBytesInRes = iconBytes;
                     iconDirEntries[validEntries].dwImageOffset = *iconOffset;
                     validEntries++;
-                    memcpy(&icons[*iconOffset], pIcon, grpIconDir->idEntries[i].dwBytesInRes);
-                    *iconOffset += grpIconDir->idEntries[i].dwBytesInRes;
+                    memcpy(&icons[*iconOffset], pIcon, iconBytes);
+                    *iconOffset += iconBytes;
                 }
                 FreeResource(hResData);
             }